From 9116817deadbc5e3494cd4f2fbb8cfb8169d0c3e Mon Sep 17 00:00:00 2001 From: Ken Simpson Date: Sun, 19 Apr 2026 11:00:48 -0700 Subject: [PATCH] fix: unwrap scalar remote prompt inputs --- README.md | 2 +- comfyui_modal_sync_cloud.py | 27 ++++++++++++++------- docs/LEARNINGS.md | 2 +- remote/modal_app.py | 27 ++++++++++++++------- tests/test_modal_executor_node.py | 39 +++++++++++++++++++++++++++++++ 5 files changed, 79 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 9c67487..9988e81 100644 --- a/README.md +++ b/README.md @@ -79,7 +79,7 @@ Remote mode requires: The normal remote path is deployed-app lookup by name. On first remote use, the extension can auto-deploy the stable cloud entrypoint from [`comfyui_modal_sync_cloud.py`](comfyui_modal_sync_cloud.py) if the configured app does not exist yet. If a previous attempt imported that cloud entrypoint only partially before failing, the loader now discards the stale cached module and retries the import cleanly before auto-deploy. -The local and remote subgraph runners also canonicalize malformed singleton-list socket indexes like `["node", [0]]` back to `["node", 0]` before executing a rewritten prompt. +The local and remote subgraph runners also canonicalize malformed singleton-list wrappers in rewritten prompts, including socket indexes like `["node", [0]]`, scalar inputs like `[4]`, and wrapped execute node ids like `["12"]`, before handing the prompt to ComfyUI. ```bash export COMFY_MODAL_EXECUTION_MODE=remote diff --git a/comfyui_modal_sync_cloud.py b/comfyui_modal_sync_cloud.py index 6d41a7c..5027076 100644 --- a/comfyui_modal_sync_cloud.py +++ b/comfyui_modal_sync_cloud.py @@ -1190,6 +1190,17 @@ def _normalize_link_output_index(value: Any) -> Any: return value +def _normalize_prompt_input_value(value: Any) -> Any: + """Unwrap transport-added singleton lists around scalar prompt input values.""" + if isinstance(value, list) and len(value) == 2 and isinstance(value[0], str): + return [value[0], _normalize_link_output_index(value[1])] + if isinstance(value, list) and len(value) == 1: + candidate = value[0] + if candidate is None or isinstance(candidate, bool | int | float | str): + return candidate + return value + + def _normalize_subgraph_payload(payload: dict[str, Any]) -> dict[str, Any]: """Return a subgraph payload with canonical prompt-link and output-index shapes.""" normalized_payload = copy.deepcopy(payload) @@ -1197,21 +1208,21 @@ def _normalize_subgraph_payload(payload: dict[str, Any]) -> dict[str, Any]: for node_info in normalized_payload.get("subgraph_prompt", {}).values(): inputs = node_info.get("inputs") or {} for input_name, input_value in list(inputs.items()): - if not isinstance(input_value, list) or len(input_value) != 2: - continue - if not isinstance(input_value[0], str): - continue - inputs[input_name] = [ - input_value[0], - _normalize_link_output_index(input_value[1]), - ] + inputs[input_name] = _normalize_prompt_input_value(input_value) for boundary_output in normalized_payload.get("boundary_outputs", []): + if "node_id" in boundary_output and isinstance(boundary_output["node_id"], list): + boundary_output["node_id"] = _normalize_prompt_input_value(boundary_output["node_id"]) if "output_index" in boundary_output: boundary_output["output_index"] = _normalize_link_output_index( boundary_output["output_index"] ) + normalized_payload["execute_node_ids"] = [ + _normalize_prompt_input_value(node_id) + for node_id in normalized_payload.get("execute_node_ids", []) + ] + return normalized_payload diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index 71a8446..7149d15 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -82,7 +82,7 @@ - Hybrid mapped components also need stream-level UI filtering. A warm PromptExecutor can still emit cached `executed` or preview UI events for static sibling nodes during per-item mapped runs, so the local relay has to drop any streamed UI event whose node id is outside the current mapped/static payload's execute and boundary-output node set. - Modal proxy nodes that sit on a mapped boundary must opt into ComfyUI `INPUT_IS_LIST` semantics themselves. Otherwise ComfyUI can auto-map the proxy once per item before Modal-Sync sees the batch, which makes the entire hybrid component rerun per item instead of letting the internal mapped scheduler split only the intended branch. - Once a proxy opts into `INPUT_IS_LIST`, ComfyUI also wraps the proxy's hidden payload input itself. The proxy has to unwrap singleton lists on `original_node_data` as well as ordinary kwargs, or it will reject valid rewritten payloads with a misleading "must be a mapping" error before remote execution even starts. -- Rewritten remote prompts need a last-mile canonicalization pass before execution. If a socket index leaks through as `["node", [0]]` or a boundary output index arrives as `[0]`, ComfyUI's executor eventually crashes trying to treat the list like an integer; normalizing those singleton wrappers on both the local and Modal-side runners avoids that failure mode. +- Rewritten remote prompts need a last-mile canonicalization pass before execution. If a socket index leaks through as `["node", [0]]`, a scalar input arrives as `[4]`, or an execute node id arrives as `["12"]`, ComfyUI's executor eventually crashes trying to treat the list like an integer or node id; normalizing those singleton wrappers on both the local and Modal-side runners avoids that failure mode. - Content-addressed sync should trust the deterministic payload path, not only the sidecar marker. If a crash happens after uploading a hash-named asset but before writing its `.done` marker, the next run should backfill the marker and reuse the existing payload instead of trying the same upload again and failing with `FileExistsError`. - The streamed Modal generator path needs the same tensor-aware serialization rules as ordinary node input/output transport. If a progress or result envelope leaks a raw `torch.Tensor`, Modal's JSON encoding fails before the local client ever sees it, so the stream layer should normalize event payloads through the shared serializer too. - Modal volume metadata calls can become a bottleneck on their own. Repeated `VolumeListFiles` checks for the same marker path are worth memoizing for the life of the queue request, and transient `ResourceExhaustedError` responses should be retried with backoff instead of surfacing immediately as hard prompt failures. diff --git a/remote/modal_app.py b/remote/modal_app.py index 6b34ed2..c0d7074 100644 --- a/remote/modal_app.py +++ b/remote/modal_app.py @@ -271,6 +271,17 @@ def _normalize_link_output_index(value: Any) -> Any: return value +def _normalize_prompt_input_value(value: Any) -> Any: + """Unwrap transport-added singleton lists around scalar prompt input values.""" + if isinstance(value, list) and len(value) == 2 and isinstance(value[0], str): + return [value[0], _normalize_link_output_index(value[1])] + if isinstance(value, list) and len(value) == 1: + candidate = value[0] + if candidate is None or isinstance(candidate, bool | int | float | str): + return candidate + return value + + def _normalize_subgraph_payload(payload: dict[str, Any]) -> dict[str, Any]: """Return a subgraph payload with canonical prompt-link and output-index shapes.""" normalized_payload = copy.deepcopy(payload) @@ -278,21 +289,21 @@ def _normalize_subgraph_payload(payload: dict[str, Any]) -> dict[str, Any]: for node_info in normalized_payload.get("subgraph_prompt", {}).values(): inputs = node_info.get("inputs") or {} for input_name, input_value in list(inputs.items()): - if not isinstance(input_value, list) or len(input_value) != 2: - continue - if not isinstance(input_value[0], str): - continue - inputs[input_name] = [ - input_value[0], - _normalize_link_output_index(input_value[1]), - ] + inputs[input_name] = _normalize_prompt_input_value(input_value) for boundary_output in normalized_payload.get("boundary_outputs", []): + if "node_id" in boundary_output and isinstance(boundary_output["node_id"], list): + boundary_output["node_id"] = _normalize_prompt_input_value(boundary_output["node_id"]) if "output_index" in boundary_output: boundary_output["output_index"] = _normalize_link_output_index( boundary_output["output_index"] ) + normalized_payload["execute_node_ids"] = [ + _normalize_prompt_input_value(node_id) + for node_id in normalized_payload.get("execute_node_ids", []) + ] + return normalized_payload diff --git a/tests/test_modal_executor_node.py b/tests/test_modal_executor_node.py index 6da2ec4..ff84749 100644 --- a/tests/test_modal_executor_node.py +++ b/tests/test_modal_executor_node.py @@ -2529,3 +2529,42 @@ def test_local_remote_app_normalizes_wrapped_subgraph_link_indexes( ) outputs = serialization_module.deserialize_node_outputs(payload) assert outputs == (10,) + + +def test_local_remote_app_normalizes_wrapped_scalar_prompt_inputs( + remote_modal_app_module: Any, + serialization_module: Any, +) -> None: + """The local fallback runner should unwrap singleton-list scalar prompt inputs.""" + payload = remote_modal_app_module.execute_subgraph_locally( + payload={ + "payload_kind": "subgraph", + "component_id": "component-1", + "subgraph_prompt": { + "remote_1": { + "class_type": "BoundarySource", + "inputs": {"value": [4]}, + "_meta": {}, + } + }, + "boundary_inputs": [], + "boundary_outputs": [ + { + "proxy_output_name": "remote_1_value", + "node_id": "remote_1", + "output_index": 0, + "io_type": "INT", + "is_list": False, + } + ], + "execute_node_ids": [["remote_1"]], + "extra_data": {}, + "custom_nodes_bundle": None, + }, + kwargs_payload="{}", + node_mapping={ + "BoundarySource": _BoundarySourceNode, + }, + ) + outputs = serialization_module.deserialize_node_outputs(payload) + assert outputs == (5,)