diff --git a/README.md b/README.md index 9a1fb1c..5c3aec4 100644 --- a/README.md +++ b/README.md @@ -211,6 +211,7 @@ Remote runtime behavior: - Ordinary split-proxy phase ordering also accounts for local feedback re-entry paths. If a later remote phase consumes a local node that traces back to an earlier remote phase's output, the earlier phase is ordered first so shared remote-only bridge values are not produced by the wrong later proxy and the rewritten local ComfyUI graph stays acyclic. - Non-returning local preview branches no longer force split proxies. If a remote sampler output feeds both a local preview branch and a later remote sampler, and that local branch never feeds back into remote work, the planner treats it as a preview tap and keeps the remote sampler chain together so downstream remote execution can continue without waiting for the local preview branch. - Preview taps also close over required non-transportable dependencies. For example, if an absorbed `VAEDecode` preview uses a `VAELoader`, the loader joins the same remote component, and any remaining local `VAE` consumers that would otherwise require exporting that loader output join as well. +- If that same component still has a local re-entry dependency, such as a remote image feeding local `BetterGrok` whose text feeds a later remote node, Modal-Sync allows ordered split proxies so the local scheduler does not see a synthetic cycle through one proxy node. - Those session refs keep values like `MODEL` remote-only while still letting the mapped proxy reconnect to them later, and the mapped proxy clears the prompt-scoped session once it finishes. - Deployed split proxies now also pass a session-affinity key into `RemoteEngine` lookup so the static and mapped halves can reuse that prompt-scoped remote state on the same worker. - Session observability now logs the full prompt-scoped lifecycle for split proxies: session creation, reuse, ref resolution, value storage, and cleanup in both local and cloud runtime paths. diff --git a/api_intercept.py b/api_intercept.py index f6a274e..6a2fd60 100644 --- a/api_intercept.py +++ b/api_intercept.py @@ -2202,6 +2202,25 @@ def _component_ancestors_of_local_source( return ancestor_node_ids +def _component_has_local_reentry_dependency( + *, + prompt: dict[str, Any], + component: RemoteComponentPlan, +) -> bool: + """Return whether a component boundary input depends on that same component's output.""" + component_node_id_set = set(component.node_ids) + for boundary_input in component.boundary_inputs: + if boundary_input.source.node_id in component_node_id_set: + continue + if _component_ancestors_of_local_source( + prompt=prompt, + source_node_id=boundary_input.source.node_id, + component_node_id_set=component_node_id_set, + ): + return True + return False + + def _order_execute_node_ids_for_transportable_splits( *, prompt: dict[str, Any], @@ -2626,12 +2645,21 @@ def _build_component_payload( if component.mapped_boundary_input_name is not None or len(component.execute_node_ids) <= 1: return None if component.local_tap_node_ids: + if not _component_has_local_reentry_dependency( + prompt=signature_prompt, + component=component, + ): + logger.info( + "Keeping remote component %s as one proxy because it contains non-returning local tap nodes %s.", + component.representative_node_id, + component.local_tap_node_ids, + ) + return None logger.info( - "Keeping remote component %s as one proxy because it contains non-returning local tap nodes %s.", + "Allowing remote component %s with local tap nodes %s to split because it has a local re-entry dependency.", component.representative_node_id, component.local_tap_node_ids, ) - return None component_node_id_set = set(component.node_ids) topological_node_ids = _subgraph_topological_node_order(component_prompt, component_node_id_set) diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index 2258ee3..40efa60 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -6,6 +6,7 @@ - Split-proxy phase ordering cannot consider only the remote component's internal links. A local feedback path such as `remote sampler A -> VAEDecode -> BetterGrok -> remote sampler B` is still a dependency from A to B when B's boundary input re-enters the remote island, and ignoring that path can make B produce shared model bridge outputs that A then depends on, creating a proxy-level cycle that the original workflow UI did not show. - Local preview branches are not the same as local feedback. If a remote output also feeds later remote work, and the local branch downstream from that output never reaches another remote node, the planner can absorb that local branch as a non-returning tap and keep the remote sampler chain in one component instead of splitting at every preview. - Absorbing a preview tap means closing over its non-transportable boundary in both directions. A local `VAEDecode` preview branch cannot be moved remote while leaving its `VAELoader` local, and once that loader is remote, any remaining local `VAEEncode` or `VAEDecode` consumers of the same `VAE` must join the component too; otherwise the planner just moves the illegal `VAE` boundary from remote input to remote output. +- Preview taps should suppress split proxies only when the final component has no local re-entry dependency. If a component output feeds a local node such as `BetterGrok` and that local node feeds back into the same remote component, a single proxy creates a synthetic `proxy -> local -> same proxy` dependency cycle, so the component still has to split into ordered phases. - Durable session bridge replay is acceptable for cheap loader/encoder recovery paths, but replaying a sampler bridge is usually a planning or affinity failure. When a downstream phase loses the live or warm bridge value for a sampler-producing component, the runtime now fails loudly instead of silently rerunning that sampler phase. - Refocus replay cannot depend only on buffered Modal websocket events. Native ComfyUI completion events such as `execution_success` may be missed while the browser is backgrounded, so the frontend now reconciles temporary Modal UI state against ComfyUI's `/queue` and `/history/{prompt_id}` endpoints before leaving old node colors or the global pill visible. diff --git a/tests/test_api_intercept.py b/tests/test_api_intercept.py index 4faa05b..749fcdb 100644 --- a/tests/test_api_intercept.py +++ b/tests/test_api_intercept.py @@ -1477,6 +1477,55 @@ def test_rewrite_keeps_local_branches_that_feed_remote_as_boundaries( ] +def test_component_local_reentry_dependency_detection( + api_intercept_module: Any, +) -> None: + """Boundary inputs that trace back to the same component require a split-capable proxy.""" + prompt = { + "1": { + "class_type": "RemoteImage", + "inputs": {}, + "_meta": {"title": "Remote Image"}, + }, + "4": { + "class_type": "LocalSink", + "inputs": {"image": ["1", 0]}, + "_meta": {"title": "Local Transform"}, + }, + "2": { + "class_type": "RemoteImageConsumer", + "inputs": {"image": ["4", 0]}, + "_meta": {"title": "Remote Consumer"}, + }, + } + component = api_intercept_module.RemoteComponentPlan( + node_ids=["1", "2"], + representative_node_id="1", + boundary_inputs=[ + api_intercept_module.BoundaryInputSpec( + proxy_input_name="remote_input_0", + source=api_intercept_module.LinkedOutputRef(node_id="4", output_index=0), + io_type="IMAGE", + targets=[ + api_intercept_module.InputTarget( + node_id="2", + input_name="image", + ) + ], + ) + ], + boundary_outputs=[], + execute_node_ids=["1", "2"], + contains_output_node=False, + local_tap_node_ids=["9"], + ) + + assert api_intercept_module._component_has_local_reentry_dependency( + prompt=prompt, + component=component, + ) + + def test_rewrite_reports_parallel_component_stages( api_intercept_module: Any, settings_module: Any,