From ee5444336ffe3c7b7c8a9370db628644eb322c82 Mon Sep 17 00:00:00 2001 From: Ken Simpson Date: Mon, 20 Apr 2026 13:04:48 -0700 Subject: [PATCH] fix: allow boundary-fed primitive lists --- README.md | 1 + comfyui_modal_sync_cloud.py | 15 +++++++++++++- docs/LEARNINGS.md | 1 + docs/UPNEXT.md | 1 + remote/modal_app.py | 21 ++++++++++++++++++-- tests/test_modal_executor_node.py | 33 +++++++++++++++++++++++++++++++ 6 files changed, 69 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 0f198cd..30ba416 100644 --- a/README.md +++ b/README.md @@ -391,6 +391,7 @@ COMFYUI_ROOT=/tmp/comfyui-modal-test/ComfyUI \ - The registry pack name is `modal-sync`, the display name is `Modal Sync`, and the current publisher id is set to `ttulttul` to match the GitHub origin owner. - [`modal_test_workflow.json`](modal_test_workflow.json) is a checked-in smoke artifact from a successful Modal-path run, not a pristine authoring workflow. - Implicit mapped execution only treats explicitly batchable transport types and list-wrapped primitive sockets as per-item inputs. List-backed semantic types like `CONDITIONING` stay broadcast across mapped item runs so the remote worker does not accidentally slice them into invalid fragments. +- Remote primitive-socket validation only rejects raw widget list literals now. If a primitive socket like `INT` is hydrated from a boundary input or upstream output and carries a list value, the runtime leaves it alone so ComfyUI can apply its normal over-list execution semantics for nodes like `NextSeeds`. ## Current limitations diff --git a/comfyui_modal_sync_cloud.py b/comfyui_modal_sync_cloud.py index 8aeb8d2..8e53d9f 100644 --- a/comfyui_modal_sync_cloud.py +++ b/comfyui_modal_sync_cloud.py @@ -2174,8 +2174,15 @@ def _coerce_prompt_primitive_input_values( def _validate_prompt_input_shapes( prompt: dict[str, Any], node_mapping: dict[str, type[Any]], + boundary_input_specs: list[dict[str, Any]] | None = None, ) -> None: """Reject prompt inputs that still look invalid for primitive widget sockets.""" + boundary_targets = { + (str(target.get("node_id")), str(target.get("input_name"))) + for boundary_input in (boundary_input_specs or []) + for target in boundary_input.get("targets", []) + if target.get("node_id") is not None and target.get("input_name") is not None + } for node_id, prompt_node in sorted(prompt.items()): class_type = str(prompt_node.get("class_type")) node_class = node_mapping.get(class_type) @@ -2194,6 +2201,8 @@ def _validate_prompt_input_shapes( and isinstance(input_value[0], str) ): continue + if (str(node_id), str(input_name)) in boundary_targets: + continue literal_value = ( input_value.get("__value__") if isinstance(input_value, dict) and "__value__" in input_value @@ -2437,7 +2446,11 @@ def _execute_subgraph_prompt( cache_type, cache_args = _prompt_executor_cache_config(execution) resolved_node_mapping = _load_nodes_module().NODE_CLASS_MAPPINGS _coerce_prompt_primitive_input_values(prompt, resolved_node_mapping) - _validate_prompt_input_shapes(prompt, resolved_node_mapping) + _validate_prompt_input_shapes( + prompt, + resolved_node_mapping, + list(normalized_payload.get("boundary_inputs", [])), + ) with ( _temporary_node_mapping(None), diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index 0098220..5a29836 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -153,3 +153,4 @@ - Once the persist phase knows a node is a `restored-hit`, that check has to happen before entry serialization and size estimation. Otherwise the code still burns CPU serializing outputs it already knows it is going to skip writing. - Distributed node-cache restore also benefits from fan-out/fan-in structure. The old per-node `await key -> await Dict get -> deserialize` loop made total restore time roughly the sum of all independent Dict reads, while batching those lookups concurrently preserves prompt-order hydration but trims the network-bound portion substantially. - Implicit mapped fan-out cannot treat every Python `list` as a batch axis. ComfyUI semantic payloads like `CONDITIONING` are also list-backed, and splitting them per item corrupts the conditioning structure into fragments like `["cond", {...}] -> "cond"` that later explode in `sampler_helpers.convert_cond`. Only explicitly batchable transport types and list-wrapped primitive sockets should participate in that automatic zipping path. +- Primitive widget sockets are not always scalar once a graph is running. Custom nodes can legally emit `list(INT)` or similar values into an `INT` input and rely on ComfyUI's normal over-list execution behavior, so remote validation should only reject list values that are still raw widget literals, not boundary-fed or upstream-fed runtime values. diff --git a/docs/UPNEXT.md b/docs/UPNEXT.md index fb7f6d6..9a416cd 100644 --- a/docs/UPNEXT.md +++ b/docs/UPNEXT.md @@ -42,5 +42,6 @@ Partial-completion semantics for one proxy would require ComfyUI scheduler behav - Online testing confirmed that `RemoteEngine(session_affinity_key)` kept the split static and mapped proxies on the same deployed Modal worker for the tested workflow. - Online testing also exposed one implicit-mapped runtime edge: list-backed `CONDITIONING` inputs must stay broadcast across mapped item runs. That regression is now covered so future batching changes do not reintroduce the `convert_cond` failure shape. +- Online testing also exposed a primitive-socket validation edge: upstream or boundary-fed `list(INT)` values are legitimate for nodes that rely on ComfyUI's over-list execution semantics, so only raw widget list literals should fail early. - Run with `COMFY_MODAL_STREAM_REMOTE_CONTAINER_LOGS=true` during broader production workflows and watch the mirrored session lifecycle logs for any real session misses under scale-out. - If production logs ever show split-proxy session misses under real scale-out, add a stronger affinity fallback or explicit leased-worker/session routing instead of relying on best-effort class-instance affinity. diff --git a/remote/modal_app.py b/remote/modal_app.py index 9782062..9e25380 100644 --- a/remote/modal_app.py +++ b/remote/modal_app.py @@ -762,8 +762,15 @@ def _coerce_prompt_primitive_input_values( def _validate_prompt_input_shapes( prompt: dict[str, Any], node_mapping: dict[str, type[Any]], + boundary_input_specs: list[dict[str, Any]] | None = None, ) -> None: """Reject prompt inputs that still look invalid for primitive widget sockets.""" + boundary_targets = { + (str(target.get("node_id")), str(target.get("input_name"))) + for boundary_input in (boundary_input_specs or []) + for target in boundary_input.get("targets", []) + if target.get("node_id") is not None and target.get("input_name") is not None + } for node_id, prompt_node in sorted(prompt.items()): class_type = str(prompt_node.get("class_type")) node_class = node_mapping.get(class_type) @@ -782,6 +789,8 @@ def _validate_prompt_input_shapes( and isinstance(input_value[0], str) ): continue + if (str(node_id), str(input_name)) in boundary_targets: + continue literal_value = ( input_value.get("__value__") if isinstance(input_value, dict) and "__value__" in input_value @@ -994,7 +1003,11 @@ def _execute_subgraph_with_mapping( hydrated_inputs=resolved_inputs, ) _coerce_prompt_primitive_input_values(prompt, node_mapping) - _validate_prompt_input_shapes(prompt, node_mapping) + _validate_prompt_input_shapes( + prompt, + node_mapping, + list(normalized_payload.get("boundary_inputs", [])), + ) required_node_ids = _resolve_required_subgraph_nodes( prompt=prompt, execute_node_ids=list(normalized_payload.get("execute_node_ids", [])), @@ -1113,7 +1126,11 @@ def _execute_subgraph_prompt( execution = _load_execution_module() resolved_node_mapping = _load_nodes_module().NODE_CLASS_MAPPINGS _coerce_prompt_primitive_input_values(prompt, resolved_node_mapping) - _validate_prompt_input_shapes(prompt, resolved_node_mapping) + _validate_prompt_input_shapes( + prompt, + resolved_node_mapping, + list(normalized_payload.get("boundary_inputs", [])), + ) with _temporary_node_mapping(node_mapping): executor = execution.PromptExecutor(_NullPromptServer()) diff --git a/tests/test_modal_executor_node.py b/tests/test_modal_executor_node.py index f1e729e..b586e42 100644 --- a/tests/test_modal_executor_node.py +++ b/tests/test_modal_executor_node.py @@ -5110,6 +5110,39 @@ def test_validate_prompt_input_shapes_rejects_list_on_primitive_socket( ) +@pytest.mark.parametrize( + ("module_fixture_name",), + [ + ("remote_modal_app_module",), + ("modal_cloud_module",), + ], +) +def test_validate_prompt_input_shapes_allows_boundary_supplied_list_on_primitive_socket( + request: Any, + module_fixture_name: str, +) -> None: + """Boundary-fed primitive lists should pass through so ComfyUI can map over them normally.""" + target_module = request.getfixturevalue(module_fixture_name) + prompt = { + "remote_1": { + "class_type": "BoundarySource", + "inputs": {"value": [4, 5]}, + "_meta": {}, + } + } + + target_module._validate_prompt_input_shapes( + prompt, + {"BoundarySource": _BoundarySourceNode}, + [ + { + "proxy_input_name": "remote_input_0", + "targets": [{"node_id": "remote_1", "input_name": "value"}], + } + ], + ) + + @pytest.mark.parametrize( ("module_fixture_name",), [