Closed superseded SeedVR2 regression test PR by pollockjj · Pull Request #20 · pollockjj/ComfyUI

pollockjj · 2026-04-27T18:50:13Z

Closed and superseded by #23.

…der regression

Copilot

Pull request overview

Adds a unit-test regression guard to ensure the SeedVR2 node’s declared schema input IDs stay aligned with the positional parameter order of its execute() method, preventing silent argument swapping if they drift.

Changes:

Add a new pytest test that compares SeedVR2InputProcessing.define_schema().inputs[*].id against inspect.signature(SeedVR2InputProcessing.execute) parameter ordering.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-27T19:30:36Z

+import inspect
+
+from comfy_extras.nodes_seedvr import SeedVR2InputProcessing
+
+
+def test_seedvr_node_signature_matches_schema():
+    schema_ids = [i.id for i in SeedVR2InputProcessing.define_schema().inputs]
+    exec_params = [p for p in inspect.signature(SeedVR2InputProcessing.execute).parameters if p != "cls"]
+    print("schema_ids:", schema_ids)
+    print("exec_params:", exec_params)
+    assert schema_ids == exec_params


This test imports comfy_extras.nodes_seedvr directly, which (via comfy_extras/nodes_seedvr.py) imports comfy.model_management; that module runs torch.cuda.is_available() at import time (see comfy/model_management.py:432). In unit tests, this can trigger CUDA initialization / GPU probing and slow down or break CPU-only CI. Consider following the existing tests-unit/comfy_extras_test/* pattern and importing under a patch.dict(sys.modules, ...) that stubs heavy modules (at least comfy.model_management, and optionally nodes/server) so the test remains a pure signature/schema check with minimal side effects.

Suggested change

import inspect

from comfy_extras.nodes_seedvr import SeedVR2InputProcessing

def test_seedvr_node_signature_matches_schema():

schema_ids = [i.id for i in SeedVR2InputProcessing.define_schema().inputs]

exec_params = [p for p in inspect.signature(SeedVR2InputProcessing.execute).parameters if p != "cls"]

print("schema_ids:", schema_ids)

print("exec_params:", exec_params)

assert schema_ids == exec_params

import importlib

import inspect

import sys

from unittest.mock import MagicMock, patch

def test_seedvr_node_signature_matches_schema():

with patch.dict(

sys.modules,

{

"comfy.model_management": MagicMock(),

"nodes": MagicMock(),

"server": MagicMock(),

},

):

sys.modules.pop("comfy_extras.nodes_seedvr", None)

seedvr_module = importlib.import_module("comfy_extras.nodes_seedvr")

SeedVR2InputProcessing = seedvr_module.SeedVR2InputProcessing

schema_ids = [i.id for i in SeedVR2InputProcessing.define_schema().inputs]

exec_params = [p for p in inspect.signature(SeedVR2InputProcessing.execute).parameters if p != "cls"]

print("schema_ids:", schema_ids)

print("exec_params:", exec_params)

assert schema_ids == exec_params

Copilot · 2026-04-27T19:30:37Z

+    print("schema_ids:", schema_ids)
+    print("exec_params:", exec_params)
+    assert schema_ids == exec_params


The print(...) statements will add noise to normal pytest output. Pytest already provides a clear diff for list equality assertions; if extra context is needed, prefer an assertion message or only emitting debug output when the assertion fails.

Suggested change

print("schema_ids:", schema_ids)

print("exec_params:", exec_params)

assert schema_ids == exec_params

assert schema_ids == exec_params, (

f"SeedVR2InputProcessing schema input ids do not match execute() "

f"parameter order: schema_ids={schema_ids}, exec_params={exec_params}"

)

…p print noise Follow-up to PR #20 review feedback and the new mydevelopment#114 plan (Production-Code Shape Independence rule): - stub comfy.model_management via unittest.mock.patch.dict instead of importing the real module, so test collection no longer triggers torch.cuda.is_available() or any GPU/server-side initialization. Live introspection confirmed comfy.model_management is the only heavy module transitively imported by comfy_extras.nodes_seedvr; nodes and server are not, so they are not stubbed. - pop and re-import comfy_extras.nodes_seedvr inside the patch context via importlib.import_module so the stubbed sys.modules entry is the one observed during attribute resolution. - replace the bare assert + print() pair with a single assert carrying a descriptive f-string failure message that includes both lists, so pytest's standard error report is the only failure surface and the passing-run stdout is quiet.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-27T20:39:13Z

+    with patch.dict(sys.modules, {"comfy.model_management": MagicMock()}):
+        sys.modules.pop("comfy_extras.nodes_seedvr", None)
+        nodes_seedvr = importlib.import_module("comfy_extras.nodes_seedvr")
+        schema_ids = [i.id for i in nodes_seedvr.SeedVR2InputProcessing.define_schema().inputs]
+        exec_params = [
+            p
+            for p in inspect.signature(nodes_seedvr.SeedVR2InputProcessing.execute).parameters
+            if p != "cls"
+        ]
+        assert schema_ids == exec_params, (
+            f"SeedVR2InputProcessing schema input ids do not match execute() "
+            f"parameter order: schema_ids={schema_ids}, exec_params={exec_params}"
+        )


Importing comfy_extras.nodes_seedvr under a stubbed comfy.model_management leaves the module cached in sys.modules with mocked globals after the patch.dict(...) context exits. That can leak into later tests (or future added tests) that import/use nodes_seedvr and accidentally run against the mocked comfy.model_management. Use a try/finally to sys.modules.pop("comfy_extras.nodes_seedvr", None) (and, if needed, clean up any comfy.model_management attribute on the comfy package) after the assertion so the mock doesn't persist outside this test.

Suggested change

with patch.dict(sys.modules, {"comfy.model_management": MagicMock()}):

sys.modules.pop("comfy_extras.nodes_seedvr", None)

nodes_seedvr = importlib.import_module("comfy_extras.nodes_seedvr")

schema_ids = [i.id for i in nodes_seedvr.SeedVR2InputProcessing.define_schema().inputs]

exec_params = [

p

for p in inspect.signature(nodes_seedvr.SeedVR2InputProcessing.execute).parameters

if p != "cls"

]

assert schema_ids == exec_params, (

f"SeedVR2InputProcessing schema input ids do not match execute() "

f"parameter order: schema_ids={schema_ids}, exec_params={exec_params}"

)

mock_model_management = MagicMock()

with patch.dict(sys.modules, {"comfy.model_management": mock_model_management}):

sys.modules.pop("comfy_extras.nodes_seedvr", None)

try:

nodes_seedvr = importlib.import_module("comfy_extras.nodes_seedvr")

schema_ids = [i.id for i in nodes_seedvr.SeedVR2InputProcessing.define_schema().inputs]

exec_params = [

p

for p in inspect.signature(nodes_seedvr.SeedVR2InputProcessing.execute).parameters

if p != "cls"

]

assert schema_ids == exec_params, (

f"SeedVR2InputProcessing schema input ids do not match execute() "

f"parameter order: schema_ids={schema_ids}, exec_params={exec_params}"

)

finally:

sys.modules.pop("comfy_extras.nodes_seedvr", None)

comfy_module = sys.modules.get("comfy")

if comfy_module is not None and getattr(comfy_module, "model_management", None) is mock_model_management:

delattr(comfy_module, "model_management")

Adopt PR #20 Copilot review on e042ff2 verbatim: wrap the stubbed import in try/finally so the mocked comfy.model_management cannot leak into later tests via the cached comfy_extras.nodes_seedvr module. - Bind the MagicMock to a local mock_model_management so the finally block can identity-check before deleting. - Pop comfy_extras.nodes_seedvr from sys.modules in finally so the next test that imports it gets a fresh load with the real comfy.model_management. - If the comfy package object itself acquired model_management as an attribute pointing at the test mock, delete that attribute so the package state matches the unstubbed condition. The assertion logic is unchanged; the test still passes on issue_114 HEAD and still fails on the spatial_overlap/temporal_tile_size swap.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-27T20:46:33Z

+`comfy.model_management` is stubbed via `patch.dict(sys.modules, ...)` so
+test collection does not transitively trigger `torch.cuda.is_available()`
+or any other GPU/server-side initialization at module-import time. Live
+introspection confirmed only `comfy.model_management` is pulled in
+transitively by `comfy_extras.nodes_seedvr` (not `nodes`, not `server`).


Docstring claims the stubbing prevents GPU init during test collection, but the stub/import happens inside the test body (during test execution). Suggest rewording to avoid implying collection-time behavior or guarantees if comfy.model_management was already imported earlier in the test run.

Suggested change

`comfy.model_management` is stubbed via `patch.dict(sys.modules, ...)` so

test collection does not transitively trigger `torch.cuda.is_available()`

or any other GPU/server-side initialization at module-import time. Live

introspection confirmed only `comfy.model_management` is pulled in

transitively by `comfy_extras.nodes_seedvr` (not `nodes`, not `server`).

`comfy.model_management` is stubbed via `patch.dict(sys.modules, ...)` for

the import performed inside this test, so importing

`comfy_extras.nodes_seedvr` here does not call

`torch.cuda.is_available()` or trigger other GPU/server-side

initialization through that dependency. Live introspection indicated that

`comfy_extras.nodes_seedvr` pulls in `comfy.model_management`

transitively here (not `nodes`, not `server`).

Copilot · 2026-04-27T20:46:34Z

+            schema_ids = [i.id for i in nodes_seedvr.SeedVR2InputProcessing.define_schema().inputs]
+            exec_params = [
+                p
+                for p in inspect.signature(nodes_seedvr.SeedVR2InputProcessing.execute).parameters


inspect.signature(...).parameters is an ordered mapping; iterating it yields parameter names implicitly. Using .parameters.keys() (or list(...)) would make it clearer that exec_params is a list of strings (not inspect.Parameter objects).

Suggested change

for p in inspect.signature(nodes_seedvr.SeedVR2InputProcessing.execute).parameters

for p in inspect.signature(nodes_seedvr.SeedVR2InputProcessing.execute).parameters.keys()

Two stylistic suggestions from Copilot's review of 5eb9257: 1. Reword the module docstring so it no longer claims the stub prevents GPU init at *test collection*; the stub/import happens inside the test body, so the wording now scopes the guarantee to the import performed inside the test. Adopted verbatim from the review. 2. Iterate inspect.signature(...).parameters.keys() instead of .parameters so it reads as 'list of names' rather than 'list of Parameter objects'. The runtime semantics are identical because .parameters yields keys when iterated, but .keys() makes the reader's job easier. Behaviour, assertion text, and stubbing strategy are unchanged. The test still passes on issue_114 HEAD and still fails on the spatial_overlap/temporal_tile_size swap.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pollockjj · 2026-04-27T21:09:20Z

+                f"parameter order: schema_ids={schema_ids}, exec_params={exec_params}"
+            )
+        finally:
+            sys.modules.pop("comfy_extras.nodes_seedvr", None)


[P2] Clear the parent package submodule cache

When this test imports comfy_extras.nodes_seedvr under the mocked comfy.model_management, Python also leaves that module on the comfy_extras.nodes_seedvr parent package attribute. The finally block only removes sys.modules, so any later test in the same process that does from comfy_extras import nodes_seedvr can receive the stale module loaded with the mock instead of re-importing the real dependency.

tests-unit: add SeedVR2InputProcessing schema-vs-execute parameter-or…

75e9bb3

…der regression

pollockjj requested a review from Copilot April 27, 2026 19:27

Copilot started reviewing on behalf of pollockjj April 27, 2026 19:27 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

pollockjj marked this pull request as ready for review April 27, 2026 20:08

pollockjj requested a review from Copilot April 27, 2026 20:34

Copilot started reviewing on behalf of pollockjj April 27, 2026 20:35 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

pollockjj requested a review from Copilot April 27, 2026 20:43

Copilot started reviewing on behalf of pollockjj April 27, 2026 20:43 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

pollockjj requested a review from Copilot April 27, 2026 20:49

Copilot started reviewing on behalf of pollockjj April 27, 2026 20:50 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

pollockjj commented Apr 27, 2026

View reviewed changes

pollockjj closed this Apr 27, 2026

pollockjj changed the title ~~Issue #114: SeedVR2 schema-vs-execute parameter-order regression test~~ Closed superseded SeedVR2 regression test PR Apr 27, 2026

pollockjj deleted the issue_114 branch May 1, 2026 16:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Closed superseded SeedVR2 regression test PR#20

Closed superseded SeedVR2 regression test PR#20
pollockjj wants to merge 4 commits into
issue_101from
issue_114

pollockjj commented Apr 27, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

pollockjj Apr 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-`comfy.model_management` is stubbed via `patch.dict(sys.modules, ...)` so
-test collection does not transitively trigger `torch.cuda.is_available()`
-or any other GPU/server-side initialization at module-import time. Live
-introspection confirmed only `comfy.model_management` is pulled in
-transitively by `comfy_extras.nodes_seedvr` (not `nodes`, not `server`).
+`comfy.model_management` is stubbed via `patch.dict(sys.modules, ...)` for
+the import performed inside this test, so importing
+`comfy_extras.nodes_seedvr` here does not call
+`torch.cuda.is_available()` or trigger other GPU/server-side
+initialization through that dependency. Live introspection indicated that
+`comfy_extras.nodes_seedvr` pulls in `comfy.model_management`
+transitively here (not `nodes`, not `server`).

	for p in inspect.signature(nodes_seedvr.SeedVR2InputProcessing.execute).parameters
	for p in inspect.signature(nodes_seedvr.SeedVR2InputProcessing.execute).parameters.keys()

Conversation

pollockjj commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

pollockjj Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pollockjj commented Apr 27, 2026 •

edited

Loading