diff --git a/README.md b/README.md index a626e45..aeba804 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,7 @@ The list above is the shortest accurate summary. If you want the execution path - Completed node progress fades out briefly instead of disappearing abruptly or remaining stuck at 100%. - Completed remote nodes use a dark blue outline and fill while later remote nodes continue executing; during finalizing, completed nodes switch to a darker blue outline with pulsing fill while Modal outputs return locally. - When a remote node starts executing, the local UI also marks its reported upstream remote ancestors complete, so quick or progress-less upstream nodes do not stay purple just because they never emitted their own completion update. + - Execution-phase ready updates no longer downgrade already completed remote nodes back to green, and ancestor detection follows links nested inside prompt input structures. - The global status pill stays in the orange waiting state while Modal capacity is still starting up and only switches to running after streamed remote execution identifies an active node or emits real node progress. - Prompt errors clear prompt-scoped remote node visual state immediately, so stale setup/ready colors do not leak into a resubmitted prompt. - Prompt success, failure, and interruption mark that prompt terminal in the frontend so late remote progress cannot resurrect stale node overlays or the global status pill. diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index ba63117..4891b8e 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -234,3 +234,4 @@ - A Modal cancellation flag can be written before the remote method body starts if the user cancels while Modal is still provisioning capacity. Remote execution registration must not clear that key on entry, and the local proxy still needs a bounded post-interrupt grace period so ComfyUI is released even when remote code is slow to observe the interrupt. - A split mapped proxy can carry an embedded `static_phase` that is executed later for mapped lane seeding. Any original boundary inputs required by that embedded phase must still be wired onto the mapped proxy node, even if they are not part of the mapped payload's top-level `boundary_inputs`; otherwise the remote worker reaches lane seeding with no hydrated value for names like `remote_input_0`. - Remote execution streams do not reliably produce explicit completion events for every node, especially quick nodes or nodes without numeric progress. The local stream bridge still has the remote subgraph prompt, so it should send dependency ancestors for each active/progress node and let the frontend mark those ancestors complete before painting the current node active. +- Coarse component-level execution status is allowed to say "these nodes are ready", but it must not downgrade same-prompt nodes that were already marked complete. Some prompt input structures also wrap links inside nested lists or dicts, so graph ancestry helpers need recursive link discovery instead of checking only top-level `[node_id, output_index]` values. diff --git a/remote/modal_app.py b/remote/modal_app.py index 1eb915d..175e38d 100644 --- a/remote/modal_app.py +++ b/remote/modal_app.py @@ -1513,6 +1513,20 @@ def _is_link(value: Any) -> bool: ) +def _iter_prompt_links(value: Any) -> Iterator[list[Any]]: + """Yield ComfyUI prompt links found inside a prompt input value.""" + if _is_link(value): + yield value + return + if isinstance(value, Mapping): + for nested_value in value.values(): + yield from _iter_prompt_links(nested_value) + return + if isinstance(value, list): + for nested_value in value: + yield from _iter_prompt_links(nested_value) + + def _normalize_link_output_index(value: Any) -> Any: """Unwrap a singleton list around a prompt-link output index when present.""" while isinstance(value, list) and len(value) == 1: @@ -2375,15 +2389,14 @@ def _remote_prompt_ancestor_node_ids(payload: dict[str, Any], node_id: str | Non if not isinstance(inputs, dict): continue for input_value in inputs.values(): - if not _is_link(input_value): - continue - upstream_node_id = str(input_value[0]) - if upstream_node_id == node_id or upstream_node_id in ancestors: - continue - if upstream_node_id not in prompt: - continue - ancestors.add(upstream_node_id) - pending.append(upstream_node_id) + for input_link in _iter_prompt_links(input_value): + upstream_node_id = str(input_link[0]) + if upstream_node_id == node_id or upstream_node_id in ancestors: + continue + if upstream_node_id not in prompt: + continue + ancestors.add(upstream_node_id) + pending.append(upstream_node_id) return sorted(ancestors) diff --git a/tests/test_modal_executor_node.py b/tests/test_modal_executor_node.py index bff0d1a..27371fd 100644 --- a/tests/test_modal_executor_node.py +++ b/tests/test_modal_executor_node.py @@ -7968,8 +7968,8 @@ def test_consume_remote_payload_stream_marks_progress_node_ancestors_complete( "extra_data": {"client_id": "client-1"}, "subgraph_prompt": { "1": {"class_type": "Loader", "inputs": {}}, - "2": {"class_type": "Condition", "inputs": {"model": ["1", 0]}}, - "3": {"class_type": "Sampler", "inputs": {"model": ["1", 0], "conditioning": ["2", 0]}}, + "2": {"class_type": "Condition", "inputs": {"model": [[["1", 0]]]}}, + "3": {"class_type": "Sampler", "inputs": {"conditioning": ["2", 0]}}, }, } stream_events = iter( diff --git a/tests/test_web_modal_toggle.py b/tests/test_web_modal_toggle.py index 3870c34..d11e51e 100644 --- a/tests/test_web_modal_toggle.py +++ b/tests/test_web_modal_toggle.py @@ -234,6 +234,15 @@ def test_streamed_modal_node_progress_completes_reported_ancestors() -> None: assert "completeRemoteAncestorsBeforeActiveNode(\n promptId,\n detail.completed_ancestor_node_ids,\n nextActiveNodeId,\n );" in source +def test_ready_updates_do_not_downgrade_completed_remote_nodes() -> None: + """Coarse execution status should not turn already completed remote nodes green again.""" + source = _modal_toggle_source() + + assert "const existingState = modalNodeStates.get(currentNodeId);" in source + assert "phase === STATE_READY &&\n existingState?.promptId === promptId" in source + assert "[STATE_COMPLETE, STATE_FINALIZING, STATE_ERROR].includes(existingState.phase)" in source + + def test_mapped_parallel_modal_progress_renders_multiple_lane_bars() -> None: """Parallel mapped Modal runs should render one local progress lane per active worker.""" source = _modal_toggle_source() diff --git a/web/modal_toggle.js b/web/modal_toggle.js index e518008..fbc24d3 100644 --- a/web/modal_toggle.js +++ b/web/modal_toggle.js @@ -715,6 +715,14 @@ function setNodesPhase(nodeIds, phase, promptId, errorMessage) { if (!shouldApplyPromptState(currentNodeId, promptId)) { continue; } + const existingState = modalNodeStates.get(currentNodeId); + if ( + phase === STATE_READY && + existingState?.promptId === promptId && + [STATE_COMPLETE, STATE_FINALIZING, STATE_ERROR].includes(existingState.phase) + ) { + continue; + } clearNodeTimer(currentNodeId); modalNodeStates.set(currentNodeId, { phase,