feat: add workflow execution state indicators to tabs#8592
Open
christian-byrne wants to merge 12 commits intomainfrom
Open
feat: add workflow execution state indicators to tabs#8592christian-byrne wants to merge 12 commits intomainfrom
christian-byrne wants to merge 12 commits intomainfrom
Conversation
christian-byrne
added
area:workflows
area:ui
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds per-workflow execution tracking and UI indicators: new execution store types/state and APIs, a composable to read/clear workflow execution state, indicator/badge components in topbar and sidebar, queue/app integrations to record outcomes, i18n strings, and unit tests. (49 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Engine as Execution Engine
participant Queue as Queue Store
participant Store as executionStore
participant Composable as useWorkflowExecutionState
participant Indicator as WorkflowExecutionIndicator
participant UI as Topbar/Sidebar
Engine->>Queue: emit job history / events
Queue->>Store: batchSetWorkflowExecutionResults(batch)
Engine->>Store: emit running/completion/error events (promptId ↔ workflowId)
Store->>Composable: update workflowExecutionStates / lastExecutionResultByWorkflowId
Composable->>Indicator: computed state -> "running"/"completed"/"error"
Indicator->>UI: render spinner/check/alert (aria-label via i18n)
UI->>Composable: clearResult(workflowId)
Composable->>Store: clearWorkflowExecutionResult(workflowId)
Store->>Composable: state -> "idle"
Indicator->>UI: hide indicator
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🎭 Playwright: ❌ 515 passed, 15 failed · 6 flaky❌ Failed Tests📊 Browser Reports
|
🎨 Storybook: ✅ Built — View Storybook |
📦 Bundle: 4.44 MB gzip 🔴 +1.21 kBDetailsSummary
Category Glance App Entry Points — 17.9 kB (baseline 17.9 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.02 MB (baseline 1.02 MB) • 🔴 +651 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.1 kB (baseline 72.1 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 435 kB (baseline 435 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 736 B (baseline 736 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 47.1 kB (baseline 47.1 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 2.56 MB (baseline 2.55 MB) • 🔴 +5.75 kBStores, services, APIs, and repositories
Status: 13 added / 13 removed Utilities & Hooks — 55.5 kB (baseline 55.5 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 12 added / 12 removed Vendor & Third-Party — 8.84 MB (baseline 8.84 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.77 MB (baseline 7.77 MB) • 🔴 +151 BBundles that do not match a named category
Status: 56 added / 56 removed |
christian-byrne
commented
Feb 3, 2026
christian-byrne
requested review from
a team and
pythongosssss
as code owners
dosubot
bot
added
the
size:L
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/topbar/WorkflowTab.vue`:
- Around line 96-108: The watch on executionState should run immediately on
mount so a pre-existing 'completed' state schedules the 5s hide; update the
watch call for executionState to include the options object { immediate: true }
(referencing the watch invocation, executionState, clearTimeoutId and
clearResult) so the handler runs once on mount and still runs on subsequent
state changes, preserving the existing timeout-creation and clearTimeout logic.
In `@src/composables/useWorkflowExecutionState.ts`:
- Around line 13-21: The computed state in useWorkflowExecutionState relies on
executionStore.promptIdToWorkflowId (and runningPromptIds) but mutations via
Map.set() won't trigger Vue ref reactivity; update
registerPromptWorkflowIdMapping()/storePrompt() to follow the immutable pattern
used for lastExecutionResultByWorkflowId by creating a new Map instance when
adding/updating entries and assigning it back to
executionStore.promptIdToWorkflowId (instead of mutating the existing Map),
which will ensure the computed in useWorkflowExecutionState (and its dependency
on promptIdToWorkflowId and runningPromptIds) re-runs and the workflow state
reflects new mappings.
🧹 Nitpick comments (1)
src/components/topbar/WorkflowExecutionIndicator.vue (1)
1-28: Prefer$tin the template and dropuseI18nhere.
tis only used in the template, so$tkeeps the script leaner and follows repo convention.♻️ Suggested refactor
<template> <i v-if="state === 'running'" class="icon-[lucide--loader-circle] size-4 shrink-0 animate-spin text-muted-foreground" - :aria-label="t('workflowExecution.running')" + :aria-label="$t('workflowExecution.running')" /> <i v-else-if="state === 'completed'" class="icon-[lucide--circle-check] size-4 shrink-0 text-jade-600" - :aria-label="t('workflowExecution.completed')" + :aria-label="$t('workflowExecution.completed')" /> <i v-else-if="state === 'error'" class="icon-[lucide--circle-alert] size-4 shrink-0 text-coral-600" - :aria-label="t('workflowExecution.error')" + :aria-label="$t('workflowExecution.error')" /> </template> <script setup lang="ts"> -import { useI18n } from 'vue-i18n' - import type { WorkflowExecutionState } from '@/composables/useWorkflowExecutionState' defineProps<{ state: WorkflowExecutionState }>() - -const { t } = useI18n() </script>Based on learnings: In Vue single-file components where the i18n t function is only used within the template, prefer using the built-in $t instead of importing useI18n.
Contributor
Author
|
Addressed all review comments:
The composable remains for convenience in components but the core logic is centralized in the store as suggested. |
christian-byrne
added
preview-gpu
dosubot
bot
added
size:XL
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/stores/executionStore.ts`:
- Around line 49-55: Change the WorkflowExecutionResult type so promptId is
optional (promptId?: string) instead of always present with an empty string;
update any other related types/usages in this file that mirror that shape (e.g.,
any other execution result/type declarations around the later block referenced)
to accept undefined and adjust code that constructs WorkflowExecutionResult
(functions or assignments that set promptId) to only include promptId when
available (remove assignments of '' and guard consumers to handle undefined).
Ensure all references to promptId in this module (including read/serialization
logic and consumers of WorkflowExecutionResult) are updated to handle an
optional string.
- Around line 766-772: The store is exporting two internals that are only used
in tests; remove lastExecutionResultByWorkflowId and getWorkflowExecutionState
from the store's public export so they remain internal implementation details,
leaving clearWorkflowExecutionResult, setWorkflowExecutionResultByWorkflowId,
and workflowExecutionStates exported as before; update tests to stop importing
those internals (either assert behavior via the public API
setWorkflowExecutionResultByWorkflowId/clearWorkflowExecutionResult/workflowExecutionStates
or access the store instance directly if absolutely necessary) so tests still
pass without exposing the internal symbols.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/stores/queueStore.ts`:
- Around line 538-556: The loop only updates execution results for
'failed'/'completed' statuses, so workflows whose latest history is non-final
keep stale indicators; after handling seenWorkflows and determining workflowId
for each job in sortedHistory, add an else branch that clears the stored result
for non-final statuses by calling
executionStore.setWorkflowExecutionResultByWorkflowId(workflowId, null, null)
(or the store's clear method if one exists) so the UI no longer shows a previous
completed/error state when the newest history entry is cancelled/ running/other.
Contributor
Author
|
Note: this is currently non-functional until https://github.com/Comfy-Org/cloud/pull/2364 is merged on cloud and the equivalent on ComfyUI. It can still be reviewed though. |
christian-byrne
added
preview-gpu
christian-byrne
force-pushed
the
feat/tab-execution-state-indicator
branch
from
5e5a900 to
9ff3b03
Compare
christian-byrne
removed
the
preview-gpu
christian-byrne
added
preview-cpu
christian-byrne
force-pushed
the
feat/tab-execution-state-indicator
branch
from
88494f7 to
ecae58b
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
- Add WorkflowExecutionResult type and lastExecutionResultByWorkflowId to executionStore - Create useWorkflowExecutionState composable for tracking execution state - Create WorkflowExecutionIndicator.vue with running/completed/error icons - Integrate indicator into WorkflowTab.vue with 5s auto-clear for completed - Add WorkflowExecutionBadge.vue for sidebar display - Add i18n accessibility labels Amp-Thread-ID: https://ampcode.com/threads/T-019c2557-1ba9-726a-8e93-978864992fd4
…ctor Replace all leftover promptId-based identifiers with their jobId equivalents after the upstream rename: - promptIdToWorkflowId → jobIdToWorkflowId - runningPromptIds → runningJobIds - Use setWorkflowExecutionResult helper instead of inline Map logic in handleExecutionSuccess Amp-Thread-ID: https://ampcode.com/threads/T-019c7a97-8947-767b-bbb7-185ccb27a17f
The upstream refactor renamed storePrompt → storeJob and nodeProgressStatesByPrompt → nodeProgressStatesByJob. Amp-Thread-ID: https://ampcode.com/threads/T-019c7a97-8947-767b-bbb7-185ccb27a17f
Add items-center to the workflow tab flex container so the execution state icons are vertically centered with the tab label text. Amp-Thread-ID: https://ampcode.com/threads/T-019c7a97-8947-767b-bbb7-185ccb27a17f
The batchResults Map already deduplicates by workflowId. The seenWorkflows Set served the same purpose and can be removed. Amp-Thread-ID: https://ampcode.com/threads/T-019c7a97-8947-767b-bbb7-185ccb27a17f
Align the type and all helpers with the upstream promptId → jobId rename for consistency. Amp-Thread-ID: https://ampcode.com/threads/T-019c7a97-8947-767b-bbb7-185ccb27a17f
When the most recent history entry for a workflow is cancelled (or other non-terminal status), clear the previous completed/error indicator instead of leaving it stale. Addresses CodeRabbit review feedback. Amp-Thread-ID: https://ampcode.com/threads/T-019c7a97-8947-767b-bbb7-185ccb27a17f
- Fix Map.set() reactivity for jobIdToWorkflowId by using immutable pattern (new Map) instead of in-place mutation - Stop exporting lastExecutionResultByWorkflowId; add getWorkflowExecutionResult() getter to minimize public API surface - Update tests to use public API (setWorkflowExecutionResultByWorkflowId) instead of directly writing to internal state Amp-Thread-ID: https://ampcode.com/threads/T-019c7daf-994a-76e5-bbac-aa5e849899ac
christian-byrne
force-pushed
the
feat/tab-execution-state-indicator
branch
from
8f663dc to
9fe7f7e
Compare
|
Related Documentation Checked 0 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
christian-byrne
added
preview-gpu
…dicators When duplicating a workflow, the cloned state retained the original's id, causing execution state indicators to appear on the duplicate instead of only on the original. By deleting state.id before loading, _configureBase() generates a fresh UUID for the duplicate. Amp-Thread-ID: https://ampcode.com/threads/T-019c837b-e368-77a9-98b2-293a462d8df6
dosubot
bot
added
size:XL
- Keep WorkflowExecutionResult/WorkflowExecutionState types (this PR) - Drop local subgraph helpers (moved to graphTraversalUtil on main) - Drop stale error overlay re-exports (now in executionErrorStore) Amp-Thread-ID: https://ampcode.com/threads/T-019c837b-e368-77a9-98b2-293a462d8df6
Keep WorkflowExecutionIndicator in ContextMenu-wrapped WorkflowTab, keep both workflowExecution and essentials/builderToolbar i18n entries, keep both execution state and missing node type tests. Amp-Thread-ID: https://ampcode.com/threads/T-019c9cf4-bad6-7159-aede-17d16949916c
⚡ Performance Report
Raw data{
"timestamp": "2026-02-27T02:54:59.135Z",
"gitSha": "82e631e42bd69c94a63fcf8c769dc00e28e33a19",
"branch": "feat/tab-execution-state-indicator",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2026.065999999986,
"styleRecalcs": 123,
"styleRecalcDurationMs": 19.290000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 384.556,
"heapDeltaBytes": -3734536
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1932.3200000000043,
"styleRecalcs": 179,
"styleRecalcDurationMs": 52.343,
"layouts": 12,
"layoutDurationMs": 3.6300000000000003,
"taskDurationMs": 861.8789999999999,
"heapDeltaBytes": -3893144
},
{
"name": "dom-widget-clipping",
"durationMs": 599.5259999999973,
"styleRecalcs": 45,
"styleRecalcDurationMs": 13.718,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 375.97099999999995,
"heapDeltaBytes": 7708436
}
]
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add visual indicators to workflow tabs showing execution state (running, completed, error).
Changes
WorkflowExecutionResulttype andlastExecutionResultByWorkflowIdstate to track execution results per workflowDesign
loader-circleicon withanimate-spin,text-muted-foregroundcircle-checkicon,text-jade-600, auto-hides after 5 secondscircle-alerticon,text-coral-600, persists until next executionReview Focus
useWorkflowExecutionState- correctly maps prompt IDs to workflow IDsWorkflowTab.vueon unmount┆Issue is synchronized with this Notion page by Unito