diff --git a/README.md b/README.md index b7874e2..4bca661 100644 --- a/README.md +++ b/README.md @@ -295,7 +295,7 @@ Independent Modal-backed components can overlap through ComfyUI's async proxy pa If a remote output also feeds both later remote work and a local branch that cannot transitively reach any later remote component, Modal-Sync splits the producer from the remote continuation. The producer publishes a small durable bridge reference; the downstream remote proxy consumes that reference immediately, while an asynchronous local materializer downloads the underlying value for the independent preview or save branch. A dispatch gate starts every nearest downstream Modal continuation before releasing the local download, so the preview still appears while later remote inference is in flight without putting transfer, rendering, or display work on its critical path. When these ordered phases place `Modal LLM` between ordinary image or video phases, the planner assigns the LLM and Comfy phases to separate reusable worker-affinity groups so a warm worker holding heavyweight Comfy models is not selected for the LLM load. Deployment protocol checks use a parameterless probe for backward compatibility, but actual execution always rebinds an affinity-aware worker handle, including after protocol compatibility has been cached. Local branches that do feed remote execution remain ordinary dependencies and preserve their original ordering. -When a component output is consumed by other remote components, Modal-Sync can keep the continuation remote. The producer returns a small durable bridge reference through the local ComfyUI proxy instead of returning the underlying `IMAGE`, `AUDIO`, `LATENT`, `MASK`, `SIGMAS`, `VIDEO`, or scalar value. A downstream remote component resolves that reference from the worker's warm bridge cache when possible, or restores it from shared Modal storage when Modal schedules the components on different containers. An independent local-only consumer gets the ordinary ComfyUI value through the parallel materializer described above. A local branch that later returns to remote execution still uses normal materialization and dependency ordering. +When a component output is consumed by other remote components, Modal-Sync can keep the continuation remote. The producer returns a small durable bridge reference through the local ComfyUI proxy instead of returning the underlying `IMAGE`, `AUDIO`, `LATENT`, `MASK`, `SIGMAS`, `VIDEO`, or scalar value. A downstream remote component resolves that reference from the worker's warm bridge cache when possible, or restores it from shared Modal storage when Modal schedules the components on different containers. An independent local-only consumer gets the ordinary ComfyUI value through the parallel materializer described above. A local branch that later returns to remote execution still uses normal materialization and dependency ordering. Deployment identity is validated through the CPU-only model stager before the first GPU dispatch. Once that protocol and fingerprint are cached as current, dispatch and speculative warmup construct only affinity-bound GPU class instances. A backward-compatible parameterless GPU probe is retained solely for older deployments that do not expose CPU identity metadata. Split proxies, mapped phases, and remote-to-remote component edges use component-local remote sessions. Durable bridge metadata and small serialized values are stored in a Modal `Dict`. Serialized bridge values above `COMFY_MODAL_BRIDGE_INLINE_MAX_BYTES` are stored as integrity-checked, content-addressed objects on the shared Modal Volume, with only the object reference retained in the Dict record. Bridge records do not retain producer inputs when the output is directly restorable or a literal node plan can rebuild it; linked loader plans retain only the boundary inputs used by their reduced dependency subgraph. The complete producer input signature still participates in the bridge key, so omitting recovery inputs cannot make distinct prompts collide. Durable writes loop until every byte is accepted, flush to storage, and verify the staged file's size and SHA-256 before publication. The producer commits new objects before publishing a completed invocation; a consumer whose mounted Volume snapshot is absent, truncated, or corrupt retries through the authoritative `Volume.read_file()` API without reloading the mount or disturbing memory-mapped models. Terminal-result diagnostics time worker publication, durable completion, invocation-record writes, Modal result yielding, local receipt, and stream close separately so post-compute stalls can be assigned to a specific transport boundary. `NOISE` remains deliberately excluded from generic serialization because ComfyUI represents it as a deferred strategy object rather than a realized tensor. Literal `RandomNoise` and `KSamplerSelect` bridge producers are instead rebuilt directly on the downstream worker, and fallback producer replay is rejected whenever its full dependency closure includes sampler work. diff --git a/comfyui_modal_sync_cloud.py b/comfyui_modal_sync_cloud.py index f087c86..fde0b33 100644 --- a/comfyui_modal_sync_cloud.py +++ b/comfyui_modal_sync_cloud.py @@ -7199,6 +7199,20 @@ def _modal_image_environment(settings: Any, runtime_fingerprint: str) -> dict[st } +def _model_stager_image_environment( + settings: Any, + runtime_fingerprint: str, +) -> dict[str, str]: + """Return deployment identity and staging values for the CPU-only helper.""" + return { + "COMFY_MODAL_APP_NAME": settings.app_name, + "COMFY_MODAL_GPU": settings.modal_gpu, + "COMFY_MODAL_REMOTE_STORAGE_ROOT": settings.remote_storage_root, + "COMFY_MODAL_RUNTIME_FINGERPRINT": runtime_fingerprint, + "HF_HUB_DISABLE_TELEMETRY": "1", + } + + def _install_remote_torch_build(image: Any, torch_build: _RemoteTorchBuild) -> Any: """Install and validate the ordered package layers for one remote Torch build.""" for layer_number, install_layer in enumerate(torch_build.install_layers, start=1): @@ -7331,10 +7345,10 @@ if modal is not None: # pragma: no branch - remote entrypoint configuration. stager_image = ( modal.Image.debian_slim(python_version=REMOTE_PYTHON_VERSION) .env( - { - "COMFY_MODAL_REMOTE_STORAGE_ROOT": settings.remote_storage_root, - "HF_HUB_DISABLE_TELEMETRY": "1", - } + _model_stager_image_environment( + settings, + runtime_identity.fingerprint, + ) ) .pip_install(*_remote_huggingface_packages()) .run_commands(_remote_huggingface_validation_command()) @@ -7500,6 +7514,19 @@ if modal is not None: # pragma: no branch - remote entrypoint configuration. raise errors[0] yield {"kind": "result", "results": results[0] if results else []} + @modal.method() + def runtime_version(self) -> dict[str, Any]: + """Return deployment identity without allocating a GPU container.""" + return { + "protocol_version": _REMOTE_APP_PROTOCOL_VERSION, + "app_name": __comfy_modal_app_name__, + "runtime_fingerprint": os.environ.get( + "COMFY_MODAL_RUNTIME_FINGERPRINT", + "", + ), + "python_version": f"{sys.version_info.major}.{sys.version_info.minor}", + } + @app.cls( **_remote_engine_cls_options( settings, diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index 39a6011..8d4d2b9 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -5,6 +5,7 @@ - A terminal local `SaveVideo` can accidentally turn a tiny encoded remote artifact into a multi-gigabyte boundary result. One observed 481-frame 768x480 video serialized 2,127,790,080 bytes of float32 RGB frames and about 5,130,752 bytes of float32 stereo audio so the local node could recreate a 2.7 MB MP4. Do not change generic VIDEO transport to a lossy encoded representation because nonterminal consumers may need exact tensors. Instead, auto-place a terminal `SaveVideo` fed entirely by remote values into the remote component and let artifact capture materialize its encoded output locally. - A split component can correctly isolate `ModalLLM` through worker affinity and still defeat warm reuse if the whole component's Comfy loader snapshot profile is stamped onto that LLM phase. Snapshot identity is also part of the parameterized Modal class identity: omit Comfy loader profiles from LLM-affinity payloads while preserving the profile on the surrounding Comfy phases. - A Modal class-parameter affinity key can still be silently defeated after the first successful protocol check if the cached-current branch returns the parameterless compatibility-probe instance. Treat protocol probes as validation-only handles: both the initial-compatible and cached-compatible paths must perform a fresh parameterized lookup before execution, and regression-test both branches. +- Modal class construction can allocate container lifecycle work before a method is called. Checking the compatibility cache inside a function that has already constructed a parameterless probe is therefore too late: consult the cache first, and construct no probe at all for a known-current runtime. An empty cache after local restart still needs identity validation, so expose the same protocol and fingerprint from the CPU-only `ModelStager`; retain the parameterless GPU probe only as compatibility fallback for older deployments. Otherwise validation can spend the spare GPU container and queue the intended affinity-bound speculative worker behind it. - Canvas decoration refactors must exercise an active node-progress render through `onDrawForeground`, not only verify palette source strings. Moving `shadowColor` into a palette object left one bare progress-panel reference that parsed successfully but raised only when the panel was painted; use the current palette color with a neutral fallback for the local-warning path. - Deliberate mixed-fanout cuts can still form a cycle in the coarse component graph when an earlier value bypasses the cut and feeds a later remote node. SCC merging must preserve graph correctness, so the repair belongs after that merge: force ordered split-proxy phases at annotated local-materializer/remote-session boundaries, carry the local materializers through the phase rewrite, and isolate LLM versus ordinary Comfy phases with distinct reusable worker-affinity groups. - ComfyUI's V3 loader assigns `RELATIVE_PYTHON_MODULE` only to nodes returned during startup. Any internal proxy or bridge class inserted into `NODE_CLASS_MAPPINGS` later must copy that loader-owned identity at the registration boundary, including cache-hit reinsertions; otherwise `/object_info` emits `python_module: null` and the frontend can abort before filtering development-only nodes. diff --git a/remote/modal_app.py b/remote/modal_app.py index 1e43050..393e048 100644 --- a/remote/modal_app.py +++ b/remote/modal_app.py @@ -6547,6 +6547,38 @@ def _remote_engine_runtime_version(remote_engine: Any) -> dict[str, Any] | None: return version_payload +def _remote_runtime_version_from_cpu_stager( + payload: Mapping[str, Any], +) -> dict[str, Any] | None: + """Read deployment identity from the CPU stager when that method is available.""" + if modal is None: + return None + settings = _settings_for_payload(payload) + deployment_app_name = modal_deployment_app_name(settings) + stager_cls = modal.Cls.from_name(deployment_app_name, "ModelStager") + stager = stager_cls() + version_method = getattr(stager, "runtime_version", None) + if version_method is None: + return None + try: + version_payload = _call_modal_method(version_method) + except AttributeError: + return None + except _modal_lookup_error_types() as exc: + if _is_missing_modal_deployment_error(exc): + raise + logger.info( + "CPU ModelStager runtime identity is unavailable for app=%s; " + "falling back to the legacy GPU protocol probe: %s", + deployment_app_name, + exc, + ) + return None + if not isinstance(version_payload, dict): + return None + return version_payload + + def _runtime_fingerprint_from_payload( version_payload: dict[str, Any] | None ) -> str | None: @@ -6778,6 +6810,55 @@ def _ensure_remote_engine_protocol_current( ) +def _lookup_protocol_current_remote_engine(payload: dict[str, Any]) -> Any: + """Return an affinity-bound engine without allocating a cached protocol probe.""" + runtime_cache_key = _modal_runtime_cache_key(payload) + with _MODAL_AUTO_DEPLOY_LOCK: + runtime_is_known_current = runtime_cache_key in _MODAL_REMOTE_APP_VERSION_OK + if runtime_is_known_current: + logger.info( + "Using cached Modal protocol validation without creating a parameterless " + "probe component=%s worker_affinity=%s.", + payload.get("component_id"), + _remote_worker_affinity_key(payload), + ) + return _lookup_deployed_remote_engine(payload) + + version_payload = _remote_runtime_version_from_cpu_stager(payload) + if version_payload is not None: + if _is_runtime_version_payload_current(version_payload, payload): + with _MODAL_AUTO_DEPLOY_LOCK: + _MODAL_REMOTE_APP_VERSION_OK.add(runtime_cache_key) + logger.info( + "Validated Modal runtime through CPU ModelStager without allocating " + "a GPU protocol probe component=%s worker_affinity=%s.", + payload.get("component_id"), + _remote_worker_affinity_key(payload), + ) + return _lookup_deployed_remote_engine(payload) + settings = _settings_for_payload(payload) + if not settings.auto_deploy: + raise ModalRemoteInvocationError( + "Deployed Modal app runtime fingerprint is out of date and " + "COMFY_MODAL_AUTO_DEPLOY=false prevents automatic replacement." + ) + return _replace_outdated_modal_app( + payload, + None, + version_payload=version_payload, + ) + + logger.info( + "CPU Modal runtime validation method is unavailable for component=%s; " + "using the legacy parameterless GPU protocol probe.", + payload.get("component_id"), + ) + return _ensure_remote_engine_protocol_current( + _lookup_deployed_remote_engine(payload, protocol_probe=True), + payload, + ) + + def _modal_auto_deploy_state( deploy_key: tuple[str, str | None], ) -> _ModalAutoDeployState: @@ -7189,13 +7270,7 @@ def _invoke_modal_warmup_blocking(warmup_request: dict[str, Any]) -> Any: deployment_app_name = modal_deployment_app_name(settings) if lookup_error_types: try: - remote_engine = _ensure_remote_engine_protocol_current( - _lookup_deployed_remote_engine( - warmup_request, - protocol_probe=True, - ), - warmup_request, - ) + remote_engine = _lookup_protocol_current_remote_engine(warmup_request) _ensure_llm_profiles_staged(warmup_request, deployment_app_name) return _invoke_remote_engine_warmup_with_recovery( remote_engine, warmup_request @@ -7217,13 +7292,7 @@ def _invoke_modal_warmup_blocking(warmup_request: dict[str, Any]) -> Any: f"Lookup failed for app={deployment_app_name!r}: {exc}." ) from exc else: - remote_engine = _ensure_remote_engine_protocol_current( - _lookup_deployed_remote_engine( - warmup_request, - protocol_probe=True, - ), - warmup_request, - ) + remote_engine = _lookup_protocol_current_remote_engine(warmup_request) _ensure_llm_profiles_staged(warmup_request, deployment_app_name) return _invoke_remote_engine_warmup_with_recovery(remote_engine, warmup_request) @@ -7967,10 +8036,7 @@ def _invoke_modal_payload_blocking( deployment_app_name = modal_deployment_app_name(settings) if lookup_error_types: try: - remote_engine = _ensure_remote_engine_protocol_current( - _lookup_deployed_remote_engine(payload, protocol_probe=True), - payload, - ) + remote_engine = _lookup_protocol_current_remote_engine(payload) _ensure_llm_profiles_staged(payload, deployment_app_name) logger.info( "Using deployed Modal app %s for component %s.", @@ -8019,10 +8085,7 @@ def _invoke_modal_payload_blocking( exc, ) else: - remote_engine = _ensure_remote_engine_protocol_current( - _lookup_deployed_remote_engine(payload, protocol_probe=True), - payload, - ) + remote_engine = _lookup_protocol_current_remote_engine(payload) _ensure_llm_profiles_staged(payload, deployment_app_name) logger.info( "Using deployed Modal app %s for component %s.", @@ -8172,11 +8235,7 @@ async def invoke_remote_engine_async( if _split_batch_boundary_inputs(payload, hydrated_inputs) is not None: if execution_mode != "local" and modal is not None: await asyncio.to_thread( - _ensure_remote_engine_protocol_current, - _lookup_deployed_remote_engine( - payload, - protocol_probe=True, - ), + _lookup_protocol_current_remote_engine, payload, ) return await _invoke_implicitly_mapped_subgraph_async( diff --git a/tests/test_modal_executor_node.py b/tests/test_modal_executor_node.py index d9340ae..c26f92b 100644 --- a/tests/test_modal_executor_node.py +++ b/tests/test_modal_executor_node.py @@ -2209,6 +2209,30 @@ def test_modal_cloud_image_environment_preserves_unique_app_name( assert image_environment["COMFY_MODAL_RUNTIME_FINGERPRINT"] == "fingerprint-1" +def test_model_stager_image_environment_preserves_deployment_identity( + modal_cloud_module: Any, +) -> None: + """The CPU protocol helper must report the exact deployed GPU runtime identity.""" + settings = types.SimpleNamespace( + app_name="comfy-modal-sync-instance", + modal_gpu="RTX-PRO-6000", + remote_storage_root="/storage", + ) + + image_environment = modal_cloud_module._model_stager_image_environment( + settings, + "fingerprint-2", + ) + + assert image_environment == { + "COMFY_MODAL_APP_NAME": "comfy-modal-sync-instance", + "COMFY_MODAL_GPU": "RTX-PRO-6000", + "COMFY_MODAL_REMOTE_STORAGE_ROOT": "/storage", + "COMFY_MODAL_RUNTIME_FINGERPRINT": "fingerprint-2", + "HF_HUB_DISABLE_TELEMETRY": "1", + } + + def test_modal_cloud_observes_distinct_workflows_for_llm_auto_mode( modal_cloud_module: Any, monkeypatch: pytest.MonkeyPatch, @@ -6315,6 +6339,96 @@ def test_remote_modal_rebinds_affinity_after_cached_protocol_validation( assert observed_payloads == [payload] +def test_remote_modal_cached_protocol_skips_parameterless_probe_construction( + remote_modal_app_module: Any, + monkeypatch: Any, +) -> None: + """A cached runtime must construct only the affinity-bound engine handle.""" + affinity_engine = object() + observed_lookups: list[tuple[dict[str, Any], bool]] = [] + + def fake_lookup( + payload: dict[str, Any], + *, + affinity_key_override: str | None = None, + protocol_probe: bool = False, + ) -> object: + """Record whether lookup attempted to allocate a parameterless probe.""" + del affinity_key_override + observed_lookups.append((payload, protocol_probe)) + return affinity_engine + + monkeypatch.setattr( + remote_modal_app_module, + "_lookup_deployed_remote_engine", + fake_lookup, + ) + payload = { + "component_id": "component-comfy::warmup:0", + "remote_worker_affinity_group": "comfy", + } + runtime_cache_key = remote_modal_app_module._modal_runtime_cache_key(payload) + remote_modal_app_module._MODAL_REMOTE_APP_VERSION_OK.clear() + remote_modal_app_module._MODAL_REMOTE_APP_VERSION_OK.add(runtime_cache_key) + try: + result = remote_modal_app_module._lookup_protocol_current_remote_engine(payload) + finally: + remote_modal_app_module._MODAL_REMOTE_APP_VERSION_OK.clear() + + assert result is affinity_engine + assert observed_lookups == [(payload, False)] + + +def test_remote_modal_uncached_protocol_uses_cpu_stager_before_gpu_lookup( + remote_modal_app_module: Any, + monkeypatch: Any, +) -> None: + """First validation should use CPU metadata and construct one bound GPU handle.""" + affinity_engine = object() + observed_lookups: list[bool] = [] + version_payload = _current_remote_runtime_payload(remote_modal_app_module) + + def fake_lookup( + payload: dict[str, Any], + *, + affinity_key_override: str | None = None, + protocol_probe: bool = False, + ) -> object: + """Record whether lookup attempted to allocate a GPU protocol probe.""" + del payload, affinity_key_override + observed_lookups.append(protocol_probe) + return affinity_engine + + monkeypatch.setattr( + remote_modal_app_module, + "_remote_runtime_version_from_cpu_stager", + lambda payload: version_payload, + ) + monkeypatch.setattr( + remote_modal_app_module, + "_lookup_deployed_remote_engine", + fake_lookup, + ) + payload = { + "component_id": "component-llm", + "remote_worker_affinity_group": "llm", + } + runtime_cache_key = remote_modal_app_module._modal_runtime_cache_key(payload) + remote_modal_app_module._MODAL_REMOTE_APP_VERSION_OK.clear() + try: + result = remote_modal_app_module._lookup_protocol_current_remote_engine(payload) + runtime_was_cached = ( + runtime_cache_key + in remote_modal_app_module._MODAL_REMOTE_APP_VERSION_OK + ) + finally: + remote_modal_app_module._MODAL_REMOTE_APP_VERSION_OK.clear() + + assert result is affinity_engine + assert runtime_was_cached is True + assert observed_lookups == [False] + + def test_workflow_gpu_changes_expected_remote_runtime_fingerprint( remote_modal_app_module: Any, ) -> None: