modular_pipeline_infrastructure model/pipeline review
Commit tested: 0f1abc4ae8b0eb2a3b40e82a310507281144c423
Review performed against the repository review rules.
Files reviewed:
src/diffusers/modular_pipelines/__init__.pysrc/diffusers/modular_pipelines/components_manager.pysrc/diffusers/modular_pipelines/mellon_node_utils.pysrc/diffusers/modular_pipelines/modular_pipeline.pysrc/diffusers/modular_pipelines/modular_pipeline_utils.py
Duplicate search: checked GitHub Issues and PRs for modular_pipeline_infrastructure, ModularPipeline, MellonPipelineConfig, from_custom_block, output_param_to_mellon_param, MODULAR_MODEL_CARD_TEMPLATE, and the specific failure modes. No exact duplicate found. Related merged PRs exist for nearby modular/Mellon work, especially #13193 and #13051.
Issue 1: ModularPipeline() crashes with an internal UnboundLocalError
Affected code:
| ifblocksisNone: |
| ifmodular_config_dictisnotNone: |
| blocks_class_name=modular_config_dict.get("_blocks_class_name") |
| else: |
| blocks_class_name=self.default_blocks_name |
| ifblocks_class_nameisnotNone: |
| diffusers_module=importlib.import_module("diffusers") |
| blocks_class=getattr(diffusers_module, blocks_class_name, None) |
| # If the blocks_class is not found or is a base class (e.g. SequentialPipelineBlocks saved by from_blocks_dict) with empty block_classes |
| # fall back to default_blocks_name |
| ifblocks_classisNoneornotblocks_class.block_classes: |
| blocks_class_name=self.default_blocks_name |
| blocks_class=getattr(diffusers_module, blocks_class_name) |
| |
| ifblocks_classisnotNone: |
| blocks=blocks_class() |
| else: |
| logger.warning(f"`blocks` is `None`, no default blocks class found for {self.__class__.__name__}") |
Problem:
When blocks=None and no default_blocks_name is available, blocks_class is never initialized, but line 1702 still reads it. The public base class therefore raises an internal UnboundLocalError instead of a clear configuration error.
Impact:
Users experimenting with custom modular blocks get a misleading crash before they can recover. This also weakens the fallback behavior added by the related merged PR #13193.
Reproduction:
fromdiffusersimportModularPipelinetry:
ModularPipeline()
exceptExceptionase:
print(type(e).__name__)
print(e)
Relevant precedent:
DiffusionPipeline and other loaders generally raise explicit ValueError/EnvironmentError messages when required pipeline metadata is missing, rather than leaking local variable errors.
Suggested fix:
blocks_class=NoneifblocksisNone:
ifmodular_config_dictisnotNone:
blocks_class_name=modular_config_dict.get("_blocks_class_name")
else:
blocks_class_name=self.default_blocks_nameifblocks_class_nameisnotNone:
diffusers_module=importlib.import_module("diffusers")
blocks_class=getattr(diffusers_module, blocks_class_name, None)
ifblocks_classisNoneornotblocks_class.block_classes:
blocks_class_name=self.default_blocks_nameblocks_class=getattr(diffusers_module, blocks_class_name, None)
ifblocks_classisnotNone:
blocks=blocks_class()
else:
raiseValueError("`blocks` must be provided when no default modular blocks class is available.")Issue 2: Mellon custom block configs drop required inputs and print debug output
Affected code:
| # Process block inputs |
| forinput_paraminblock.inputs: |
| ifinput_param.nameisNone: |
| continue |
| ifinput_param.nameininput_types: |
| input_param=copy.copy(input_param) |
| input_param.metadata= {"mellon": input_types[input_param.name]} |
| print(f" processing input: {input_param.name}, metadata: {input_param.metadata}") |
| inputs.append(input_param_to_mellon_param(input_param)) |
| |
| # Process block outputs |
| foroutput_paraminblock.outputs: |
| ifoutput_param.nameisNone: |
| continue |
| ifoutput_param.nameinoutput_types: |
| output_param=copy.copy(output_param) |
| output_param.metadata= {"mellon": output_types[output_param.name]} |
| outputs.append(output_param_to_mellon_param(output_param)) |
| |
| # Process expected components (all map to model inputs) |
| component_names=block.component_names |
| forcomponent_nameincomponent_names: |
| model_inputs.append(MellonParam.Input.model(component_name)) |
| |
| # Always add doc output |
| outputs.append(MellonParam.doc()) |
| |
| node_spec= { |
| "inputs": inputs, |
| "model_inputs": model_inputs, |
| "outputs": outputs, |
| "required_inputs": [], |
| "required_model_inputs": [], |
| "block_name": "custom", |
Problem:
MellonPipelineConfig.from_custom_block() ignores InputParam.required=True and always emits "required_inputs": []. The same loop also prints processing input: ... to stdout.
Impact:
Generated Mellon configs do not mark required custom block inputs, so the UI schema is less accurate than the modular block contract. The stdout print also leaks debug noise from a library API.
Reproduction:
fromdiffusersimportInputParam, ModularPipelineBlocks, OutputParamfromdiffusers.modular_pipelines.mellon_node_utilsimportMellonPipelineConfigclassRequiredPromptBlock(ModularPipelineBlocks):
@propertydefinputs(self):
return [InputParam("prompt", type_hint=str, required=True, metadata={"mellon": "textbox"})]
@propertydefintermediate_outputs(self):
return [OutputParam("prompt", type_hint=str, metadata={"mellon": "text"})]
cfg=MellonPipelineConfig.from_custom_block(RequiredPromptBlock())
print(cfg.node_params["custom"]["params"]["prompt"])Relevant precedent:
node_spec_to_mellon_dict() already supports required_inputs and marks labels via mark_required().
Suggested fix:
required_inputs= []
forinput_paraminblock.inputs:
ifinput_param.nameisNone:
continueifinput_param.nameininput_types:
input_param=copy.copy(input_param)
input_param.metadata= {"mellon": input_types[input_param.name]}
ifinput_param.required:
required_inputs.append(input_param.name)
inputs.append(input_param_to_mellon_param(input_param))
node_spec= {
"inputs": inputs,
"model_inputs": model_inputs,
"outputs": outputs,
"required_inputs": required_inputs,
"required_model_inputs": [p.nameforpinmodel_inputs],
"block_name": "custom",
}Issue 3: Mellon output metadata ignores explicit MellonParam instances
Affected code:
| defoutput_param_to_mellon_param(output_param: "OutputParam") ->MellonParam: |
| """ |
| Convert an OutputParam to a MellonParam using metadata. |
| |
| Args: |
| output_param: An OutputParam with optional metadata={"mellon": "<type>"} where type is one of: |
| image, video, text, model. If metadata is None or unknown, maps to "custom". |
| |
| Returns: |
| MellonParam instance |
| """ |
| name=output_param.name |
| metadata=output_param.metadata |
| mellon_type=metadata.get("mellon") ifmetadataelseNone |
| |
| ifmellon_type=="image": |
| returnMellonParam.Output.image(name) |
| elifmellon_type=="video": |
| returnMellonParam.Output.video(name) |
| elifmellon_type=="text": |
| returnMellonParam.Output.text(name) |
| elifmellon_type=="model": |
| returnMellonParam.Output.model(name) |
| else: |
| # None or unknown -> custom |
| returnMellonParam.Output.custom_type(name, type="custom") |
Problem:
input_param_to_mellon_param() accepts metadata={"mellon": MellonParam(...)}, but output_param_to_mellon_param() only handles string metadata. Passing a fully custom MellonParam.Output.* silently falls through to "custom".
Impact:
The docs promise full-control Mellon metadata for parameters, but custom output UI metadata is lost.
Reproduction:
fromdiffusersimportOutputParamfromdiffusers.modular_pipelines.mellon_node_utilsimportMellonParam, output_param_to_mellon_paramparam=OutputParam(
"answer",
type_hint=str,
metadata={"mellon": MellonParam.Output.text("answer")},
)
print(output_param_to_mellon_param(param).to_dict())Relevant precedent:
| # If it's already a MellonParam, return it directly |
| ifisinstance(mellon_value, MellonParam): |
| returnmellon_value |
Suggested fix:
mellon_value=metadata.get("mellon") ifmetadataelseNoneifisinstance(mellon_value, MellonParam):
returnmellon_valuemellon_type=mellon_valueIssue 4: Pushed modular model cards contain a [TODO] placeholder
Affected code:
| # Template for modular pipeline model card description with placeholders |
| MODULAR_MODEL_CARD_TEMPLATE="""{model_description} |
| |
| ## Example Usage |
| |
| [TODO] |
| |
| ## Pipeline Architecture |
| |
| This modular pipeline is composed of the following blocks: |
| |
| {blocks_description} {trigger_inputs_section} |
| |
| ## Model Components |
| |
| {components_description} {configs_section} |
| |
| {io_specification_section} |
| """ |
| ifpush_to_hub: |
| card_content=generate_modular_model_card_content(self.blocks) |
| model_card=load_or_create_model_card( |
| repo_id, |
| token=token, |
| is_pipeline=True, |
| model_description=MODULAR_MODEL_CARD_TEMPLATE.format(**card_content), |
| is_modular=True, |
| update_model_card=update_model_card, |
| ) |
| model_card=populate_model_card(model_card, tags=card_content["tags"]) |
| model_card.save(os.path.join(save_directory, "README.md")) |
Problem:
MODULAR_MODEL_CARD_TEMPLATE hardcodes [TODO] under “Example Usage”. ModularPipeline.save_pretrained(push_to_hub=True) formats this template directly into README.md.
Impact:
Every pushed modular pipeline model card can publish placeholder text, violating the review rule to avoid TODO placeholders in generated user-facing artifacts.
Reproduction:
fromdiffusersimportModularPipelineBlocksfromdiffusers.modular_pipelines.modular_pipeline_utilsimport (
MODULAR_MODEL_CARD_TEMPLATE,
generate_modular_model_card_content,
)
classEmptyBlocks(ModularPipelineBlocks):
passreadme=MODULAR_MODEL_CARD_TEMPLATE.format(**generate_modular_model_card_content(EmptyBlocks()))
print("[TODO]"inreadme)Relevant precedent:
The modular auto-doc rule requires generated docs to avoid unresolved TODO placeholders.
Suggested fix:
MODULAR_MODEL_CARD_TEMPLATE="""{model_description}## Pipeline ArchitectureThis modular pipeline is composed of the following blocks:{blocks_description} {trigger_inputs_section}## Model Components{components_description} {configs_section}{io_specification_section}"""Issue 5: Mellon guide uses the wrong save() argument name
Affected code:
| mellon_config = MellonPipelineConfig.from_custom_block(blocks) |
| # push the default template to `repo_id`, you will need to pass the same local folder path so that it will save the config locally first |
| mellon_config.save( |
| local_dir="/path/local/folder", |
| repo_id= repo_id, |
| push_to_hub=True |
| ) |
Problem:
The guide calls mellon_config.save(local_dir=...), but MellonPipelineConfig.save() requires the first argument as save_directory and does not accept local_dir.
Impact:
The documented copy-paste path for generating and pushing a Mellon config fails immediately.
Reproduction:
fromdiffusers.modular_pipelines.mellon_node_utilsimportMellonPipelineConfigcfg=MellonPipelineConfig(node_specs={})
try:
cfg.save(local_dir="somewhere", repo_id="user/repo", push_to_hub=False)
exceptExceptionase:
print(type(e).__name__)
print(e)Relevant precedent:
The merged Mellon utility PR used save_directory=local_dir in its helper script: #13051
Suggested fix:
mellon_config.save(
save_directory="/path/local/folder",
repo_id=repo_id,
push_to_hub=True,
)
Coverage Status
Fast modular pipeline tests exist under tests/modular_pipelines/, and one slow/nightly custom-block integration exists at:
| @slow |
| @nightly |
| @require_torch |
| classTestKreaCustomBlocksIntegration: |
| repo_id="krea/krea-realtime-video" |
| |
| deftest_loading_from_hub(self): |
| blocks=ModularPipelineBlocks.from_pretrained(self.repo_id, trust_remote_code=True) |
| block_names=sorted(blocks.sub_blocks) |
| |
| assertblock_names==sorted(["text_encoder", "before_denoise", "denoise", "decode"]) |
| |
| pipe=WanModularPipeline(blocks, self.repo_id) |
| pipe.load_components( |
| trust_remote_code=True, |
| device_map="cuda", |
| torch_dtype={"default": torch.bfloat16, "vae": torch.float16}, |
| ) |
| assertlen(pipe.components) ==7 |
| assertsorted(pipe.components) ==sorted( |
| ["text_encoder", "tokenizer", "guider", "scheduler", "vae", "transformer", "video_processor"] |
| ) |
| |
| deftest_forward(self): |
| blocks=ModularPipelineBlocks.from_pretrained(self.repo_id, trust_remote_code=True) |
| pipe=WanModularPipeline(blocks, self.repo_id) |
| pipe.load_components( |
| trust_remote_code=True, |
| device_map="cuda", |
| torch_dtype={"default": torch.bfloat16, "vae": torch.float16}, |
| ) |
| |
| num_frames_per_block=2 |
| num_blocks=2 |
| |
| state=PipelineState() |
| state.set("frame_cache_context", deque(maxlen=pipe.config.frame_cache_len)) |
| |
| prompt= ["a cat sitting on a boat"] |
| |
| forblockinpipe.transformer.blocks: |
| block.self_attn.fuse_projections() |
| |
| forblock_idxinrange(num_blocks): |
| state=pipe( |
I did not find dedicated fast tests for mellon_node_utils.py; that gap covers Issues 2, 3, and 5. No missing slow-test report item is raised because slow modular infrastructure coverage is present.
modular_pipeline_infrastructuremodel/pipeline reviewCommit tested:
0f1abc4ae8b0eb2a3b40e82a310507281144c423Review performed against the repository review rules.
Files reviewed:
src/diffusers/modular_pipelines/__init__.pysrc/diffusers/modular_pipelines/components_manager.pysrc/diffusers/modular_pipelines/mellon_node_utils.pysrc/diffusers/modular_pipelines/modular_pipeline.pysrc/diffusers/modular_pipelines/modular_pipeline_utils.pyDuplicate search: checked GitHub Issues and PRs for
modular_pipeline_infrastructure,ModularPipeline,MellonPipelineConfig,from_custom_block,output_param_to_mellon_param,MODULAR_MODEL_CARD_TEMPLATE, and the specific failure modes. No exact duplicate found. Related merged PRs exist for nearby modular/Mellon work, especially #13193 and #13051.Issue 1:
ModularPipeline()crashes with an internalUnboundLocalErrorAffected code:
diffusers/src/diffusers/modular_pipelines/modular_pipeline.py
Lines 1688 to 1705 in 0f1abc4
Problem:
When
blocks=Noneand nodefault_blocks_nameis available,blocks_classis never initialized, but line 1702 still reads it. The public base class therefore raises an internalUnboundLocalErrorinstead of a clear configuration error.Impact:
Users experimenting with custom modular blocks get a misleading crash before they can recover. This also weakens the fallback behavior added by the related merged PR #13193.
Reproduction:
Relevant precedent:
DiffusionPipelineand other loaders generally raise explicitValueError/EnvironmentErrormessages when required pipeline metadata is missing, rather than leaking local variable errors.Suggested fix:
Issue 2: Mellon custom block configs drop required inputs and print debug output
Affected code:
diffusers/src/diffusers/modular_pipelines/mellon_node_utils.py
Lines 1062 to 1095 in 0f1abc4
Problem:
MellonPipelineConfig.from_custom_block()ignoresInputParam.required=Trueand always emits"required_inputs": []. The same loop also printsprocessing input: ...to stdout.Impact:
Generated Mellon configs do not mark required custom block inputs, so the UI schema is less accurate than the modular block contract. The stdout print also leaks debug noise from a library API.
Reproduction:
Relevant precedent:
node_spec_to_mellon_dict()already supportsrequired_inputsand marks labels viamark_required().Suggested fix:
Issue 3: Mellon output metadata ignores explicit
MellonParaminstancesAffected code:
diffusers/src/diffusers/modular_pipelines/mellon_node_utils.py
Lines 466 to 491 in 0f1abc4
Problem:
input_param_to_mellon_param()acceptsmetadata={"mellon": MellonParam(...)}, butoutput_param_to_mellon_param()only handles string metadata. Passing a fully customMellonParam.Output.*silently falls through to"custom".Impact:
The docs promise full-control Mellon metadata for parameters, but custom output UI metadata is lost.
Reproduction:
Relevant precedent:
diffusers/src/diffusers/modular_pipelines/mellon_node_utils.py
Lines 439 to 441 in 0f1abc4
Suggested fix:
Issue 4: Pushed modular model cards contain a
[TODO]placeholderAffected code:
diffusers/src/diffusers/modular_pipelines/modular_pipeline_utils.py
Lines 38 to 56 in 0f1abc4
diffusers/src/diffusers/modular_pipelines/modular_pipeline.py
Lines 2007 to 2018 in 0f1abc4
Problem:
MODULAR_MODEL_CARD_TEMPLATEhardcodes[TODO]under “Example Usage”.ModularPipeline.save_pretrained(push_to_hub=True)formats this template directly intoREADME.md.Impact:
Every pushed modular pipeline model card can publish placeholder text, violating the review rule to avoid TODO placeholders in generated user-facing artifacts.
Reproduction:
Relevant precedent:
The modular auto-doc rule requires generated docs to avoid unresolved TODO placeholders.
Suggested fix:
Issue 5: Mellon guide uses the wrong
save()argument nameAffected code:
diffusers/docs/source/en/modular_diffusers/mellon.md
Lines 154 to 160 in 0f1abc4
Problem:
The guide calls
mellon_config.save(local_dir=...), butMellonPipelineConfig.save()requires the first argument assave_directoryand does not acceptlocal_dir.Impact:
The documented copy-paste path for generating and pushing a Mellon config fails immediately.
Reproduction:
Relevant precedent:
The merged Mellon utility PR used
save_directory=local_dirin its helper script: #13051Suggested fix:
Coverage Status
Fast modular pipeline tests exist under
tests/modular_pipelines/, and one slow/nightly custom-block integration exists at:diffusers/tests/modular_pipelines/test_modular_pipelines_custom_blocks.py
Lines 581 to 625 in 0f1abc4
I did not find dedicated fast tests for
mellon_node_utils.py; that gap covers Issues 2, 3, and 5. No missing slow-test report item is raised because slow modular infrastructure coverage is present.