From 4e5abe68d0dede43e2009d8dc83cd67479a16c15 Mon Sep 17 00:00:00 2001 From: Ken Simpson Date: Thu, 23 Apr 2026 16:50:16 -0700 Subject: [PATCH] fix: clear modal ui on prompt interruption --- docs/LEARNINGS.md | 1 + tests/modal_toggle_frontend.test.mjs | 52 ++++++++++++++++++++++++++++ tests/test_web_modal_toggle.py | 13 +++++++ web/modal_toggle.js | 23 ++++++++++-- 4 files changed, 87 insertions(+), 2 deletions(-) diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index 856cf38..7782d4a 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -3,6 +3,7 @@ ## 2026-04-23 - Nested workflow-marker resolution has to stop at the first matching composed prompt-id suffix, not union every suffix. If a subgraph node `195:27` is marked remote and the prompt also contains an unrelated root node `27`, collecting both matches makes the real nested node look local during boundary validation and proxy rewrite. +- Frontend interruption cleanup has to key off `prompt_id`, not the native execution event's node fields. ComfyUI can emit `execution_interrupted` without a representative remote node id, and if Modal UI teardown depends on resolving that node the cancelled prompt stays visually stuck in ready/active state even though interrupt propagation worked. ## 2026-04-22 diff --git a/tests/modal_toggle_frontend.test.mjs b/tests/modal_toggle_frontend.test.mjs index 3388997..b1a93e9 100644 --- a/tests/modal_toggle_frontend.test.mjs +++ b/tests/modal_toggle_frontend.test.mjs @@ -43,7 +43,9 @@ const transformedSource = `${[ " registerPromptComponents,", " resolveComponentNodeIds,", " handleModalProgress,", + " handleModalStatus,", " handleExecutionPhase,", + " handlePromptInterruption,", " clearPromptRemoteStates,", " getRemoteVisualState,", " modalNodeStates,", @@ -202,3 +204,53 @@ modalToggle.handleModalProgress({ }, }); assert.equal(modalToggle.getRemoteVisualState({ id: "11" })?.phase, modalToggle.STATE_READY); + +resetFrontendState(); +modalToggle.registerPromptComponents("prompt-e", ["10", "11"], [ + { + representative_node_id: "10", + node_ids: ["10", "11"], + }, +]); +modalToggle.handleExecutionPhase( + { + detail: { + prompt_id: "prompt-e", + node: "10", + }, + }, + modalToggle.EXECUTION_PHASE, +); +assert.equal(modalToggle.modalPromptStates.has("prompt-e"), true); +assert.equal(modalToggle.modalNodeStates.get("10")?.phase, modalToggle.STATE_READY); +modalToggle.handlePromptInterruption("prompt-e"); +assert.equal(modalToggle.modalPromptStates.has("prompt-e"), false); +assert.equal(modalToggle.modalNodeStates.has("10"), false); +assert.equal(modalToggle.modalNodeStates.has("11"), false); + +resetFrontendState(); +modalToggle.registerPromptComponents("prompt-f", ["20", "21"], [ + { + representative_node_id: "20", + node_ids: ["20", "21"], + }, +]); +modalToggle.handleExecutionPhase( + { + detail: { + prompt_id: "prompt-f", + node: "20", + }, + }, + modalToggle.EXECUTION_PHASE, +); +modalToggle.handleModalStatus({ + detail: { + prompt_id: "prompt-f", + phase: "execution_interrupted", + node_ids: ["20", "21"], + }, +}); +assert.equal(modalToggle.modalPromptStates.has("prompt-f"), false); +assert.equal(modalToggle.modalNodeStates.has("20"), false); +assert.equal(modalToggle.modalNodeStates.has("21"), false); diff --git a/tests/test_web_modal_toggle.py b/tests/test_web_modal_toggle.py index 21e934f..6115e0f 100644 --- a/tests/test_web_modal_toggle.py +++ b/tests/test_web_modal_toggle.py @@ -62,6 +62,19 @@ def test_remote_modal_status_tracks_active_node_ids() -> None: assert "clearPromptRemoteStates(promptId)" in source +def test_interrupted_prompts_clear_modal_ui_by_prompt_id() -> None: + """Interrupt cleanup should not depend on native execution events naming a specific node.""" + source = _modal_toggle_source() + + assert 'if (detail.phase === "execution_interrupted") {' in source + assert "function handlePromptInterruption(promptId)" in source + assert "clearGlobalStatusPhase(promptId);" in source + assert "clearPromptRemoteStates(promptId);" in source + assert 'api.addEventListener("execution_interrupted", (event) => {' in source + assert "const promptId = String(eventDetail(event).prompt_id ?? \"\");" in source + assert "handlePromptInterruption(promptId);" in source + + def test_remote_modal_uses_distinct_ready_active_and_complete_colors() -> None: """The frontend should distinguish ready, active, and completed remote nodes visually.""" source = _modal_toggle_source() diff --git a/web/modal_toggle.js b/web/modal_toggle.js index f347cba..4d79df9 100644 --- a/web/modal_toggle.js +++ b/web/modal_toggle.js @@ -1717,6 +1717,12 @@ function handleModalStatus(event) { return; } + if (detail.phase === "execution_interrupted") { + endSyntheticExecutionUi(promptId); + handlePromptInterruption(promptId); + return; + } + if (detail.phase === EXECUTION_PHASE) { endSyntheticExecutionUi(promptId); const nextActiveNodeId = @@ -1748,6 +1754,18 @@ function handleModalStatus(event) { } } +/** + * Clear one interrupted prompt's temporary Modal UI state. + * @param {string} promptId + */ +function handlePromptInterruption(promptId) { + if (!promptId) { + return; + } + clearGlobalStatusPhase(promptId); + clearPromptRemoteStates(promptId); +} + /** * Apply a streamed numeric Modal node-progress event. * @param {CustomEvent} event @@ -2037,8 +2055,9 @@ function registerExecutionListeners() { handleExecutionPhase(event, STATE_ERROR); }); api.addEventListener("execution_interrupted", (event) => { - endSyntheticExecutionUi(String(eventDetail(event).prompt_id ?? ""), true); - handleExecutionPhase(event, STATE_ERROR); + const promptId = String(eventDetail(event).prompt_id ?? ""); + endSyntheticExecutionUi(promptId); + handlePromptInterruption(promptId); }); api.addEventListener("execution_success", (event) => { const detail = eventDetail(event);