fix: allow boundary-fed primitive lists
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
+19
-2
@@ -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())
|
||||
|
||||
@@ -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",),
|
||||
[
|
||||
|
||||
Reference in New Issue
Block a user