Feat: Persist all unsaved workflow tabs #6050
Conversation
|
Just a little worried about giving the client more ownership over the workflow state - given future plans for possible CRDT (in particular, syncing when going offline->online) and some other features, but in general it should be fine. |
christian-byrne
added
area:workflows
claude-review
jtydhr88
added
claude-review
Yourz
force-pushed
the
workflow-persistence
branch
from
5bbaf17 to
eaf1b2c
Compare
📝 WalkthroughWalkthroughIntroduces draft-based workflow persistence: an LRU draft cache persisted to localStorage, a Pinia draft store with save/load/move/remove semantics and quota handling, and integration into workflow persistence, store, and service layers to save and restore open/temporary workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant DraftStore
participant LocalStorage
participant WorkflowStore
User->>App: Trigger save (manual/auto)
App->>DraftStore: saveDraft(path, snapshot)
activate DraftStore
DraftStore->>LocalStorage: write draft (Comfy.Workflow.Drafts)
alt QuotaExceeded
DraftStore->>DraftStore: evict oldest draft (LRU)
DraftStore->>LocalStorage: retry write
end
DraftStore-->>App: confirm draft saved
deactivate DraftStore
User->>App: Restart / open app
App->>DraftStore: loadPersistedWorkflow(preferredPath, fallbackToLatestDraft)
activate DraftStore
DraftStore->>LocalStorage: read drafts & order
DraftStore->>DraftStore: parse/validate draft, mark used
alt Draft available and valid
DraftStore->>WorkflowStore: provide draft content
WorkflowStore-->>App: open workflow tab (marked modified if draft applied)
else No valid draft
DraftStore-->>App: fallback (session/localStorage or none)
end
deactivate DraftStore
App->>User: restore open workflows/tabs
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎭 Playwright Tests:
|
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/26/2026, 06:08:14 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 22.8 kB (baseline 22.8 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 958 kB (baseline 957 kB) • 🔴 +1.65 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 80.7 kB (baseline 80.7 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 466 kB (baseline 466 kB) • 🟢 -8 BConfiguration panels, inspectors, and settings screens
Status: 12 added / 12 removed User & Accounts — 3.94 kB (baseline 3.94 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 3 added / 3 removed Editors & Dialogs — 2.83 kB (baseline 2.83 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 33.7 kB (baseline 33.7 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 3.19 MB (baseline 3.18 MB) • 🔴 +6.8 kBStores, services, APIs, and repositories
Status: 8 added / 8 removed Utilities & Hooks — 25.2 kB (baseline 25.2 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 7 added / 7 removed Vendor & Third-Party — 10.7 MB (baseline 10.7 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 6.49 MB (baseline 6.49 MB) • 🟢 -192 BBundles that do not match a named category
Status: 34 added / 34 removed |
Yourz
force-pushed
the
workflow-persistence
branch
2 times, most recently
from
697beec to
21c480d
Compare
dosubot
bot
added
the
size:XL
Yourz
changed the title
coderabbitai
bot
requested review from
KarryCharon,
Yorha4D,
comfyui-wiki and
shinshin86
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/platform/workflow/persistence/base/draftCache.ts`:
- Around line 26-44: upsertDraft can produce an orphaned draft when called with
limit=0; add a minimum-limit guard at the start of upsertDraft (e.g., coerce
limit = Math.max(1, limit) or throw for invalid limits) so the eviction loop can
never empty order while leaving a draft in drafts; update references in the
function (touchEntry, the eviction while loop, and the final
createDraftCacheState call) to use the sanitized limit value.
In `@src/platform/workflow/persistence/composables/useWorkflowPersistence.ts`:
- Around line 192-197: The code unsafely casts JSON.parse results for
getStorageValue('Comfy.OpenWorkflowsPaths') and
getStorageValue('Comfy.ActiveWorkflowIndex') to string[] and number respectively
(storedWorkflows, storedActiveIndex); instead add runtime validation after
parsing: ensure storedWorkflows is an array of strings (Array.isArray and every
item typeof === 'string') and default to [] if invalid, and ensure
storedActiveIndex is a finite number (typeof === 'number' and Number.isFinite)
and default to -1 if invalid; update any downstream usage to rely on these
validated values.
In `@src/platform/workflow/persistence/stores/workflowDraftStore.ts`:
- Around line 82-97: Update the tryLoadGraph function call to include inline
parameter comments for the boolean args of comfyApp.loadGraphData so it's clear
what they control: annotate the second argument as /* clean= */ and the third as
/* restore_view= */ while keeping the same values and order (i.e.,
comfyApp.loadGraphData(workflow, /* clean= */ true, /* restore_view= */ true,
workflowName)); locate this in tryLoadGraph in workflowDraftStore.ts and apply
the same commenting pattern used in workflowService.ts.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
browser_tests/tests/colorPalette.spec.tssrc/platform/workflow/management/stores/workflowStore.test.tssrc/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/persistence/base/draftCache.tssrc/platform/workflow/persistence/composables/useWorkflowPersistence.tssrc/platform/workflow/persistence/stores/workflowDraftStore.tstests-ui/tests/platform/workflow/persistence/draftCache.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts
🧰 Additional context used
📓 Path-based instructions (14)
+(tests-ui|src)/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
+(tests-ui|src)/**/*.test.ts: Unit and component tests belong intests-ui/orsrc/**/*.test.tsusing Vitest
Write tests for all changes, especially bug fixes to catch future regressions
Do not write tests dependent on non-behavioral features like utility classes or styles
Do not write tests that just test the mocks - ensure tests fail when code behaves unexpectedly
Leverage Vitest's utilities for mocking where possible
Keep module mocks contained - do not use global mutable state within test files; usevi.hoisted()if necessary
Use Vue Test Utils for Component testing and follow best practices for making components easy to test
Aim for behavioral coverage of critical and new features in unit tests
Files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tssrc/platform/workflow/management/stores/workflowStore.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts
+(tests-ui|src|browser_tests)/**/*.+(test.ts|spec.ts)
📄 CodeRabbit inference engine (AGENTS.md)
+(tests-ui|src|browser_tests)/**/*.+(test.ts|spec.ts): Do not write change detector tests - avoid tests that only assert default values
Be parsimonious in testing - do not write redundant tests
Don't Mock What You Don't Own
Files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tsbrowser_tests/tests/colorPalette.spec.tssrc/platform/workflow/management/stores/workflowStore.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts
browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (browser_tests/CLAUDE.md)
browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}: Test user workflows in browser tests
Use Playwright fixtures for browser tests
Follow naming conventions for browser tests
Check assets/ directory for test data when writing tests
Prefer specific selectors in browser tests
Test across multiple viewports
Files:
browser_tests/tests/colorPalette.spec.ts
browser_tests/**/*.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
browser_tests/**/*.spec.ts: E2E tests use Playwright and belong inbrowser_tests/**/*.spec.ts
Follow Playwright best practices for E2E testing
Do not use waitForTimeout in Playwright tests - use Locator actions and retrying assertions instead
Use tags like@mobile,@2xin Playwright tests for configuration-aware test selection
Files:
browser_tests/tests/colorPalette.spec.ts
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/platform/workflow/persistence/base/draftCache.tssrc/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/persistence/stores/workflowDraftStore.tssrc/platform/workflow/management/stores/workflowStore.test.tssrc/platform/workflow/persistence/composables/useWorkflowPersistence.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
src/**/*.ts: Derive component types usingvue-component-type-helpers(ComponentProps,ComponentSlots) instead of separate type files
Use es-toolkit for utility functions
Minimize the surface area (exported values) of each module and composable
Favor pure functions, especially testable ones
Files:
src/platform/workflow/persistence/base/draftCache.tssrc/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/persistence/stores/workflowDraftStore.tssrc/platform/workflow/management/stores/workflowStore.test.tssrc/platform/workflow/persistence/composables/useWorkflowPersistence.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using@ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
src/**/*.{ts,tsx,vue}: Use separateimport typestatements instead of inlinetypein mixed imports
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, 80-character width
Sort and group imports by plugin, runpnpm formatbefore committing
Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Write code that is expressive and self-documenting - avoid unnecessary comments
Do not add or retain redundant comments - clean as you go
Avoid mutable state - prefer immutability and assignment at point of declaration
Watch out for Code Smells and refactor to avoid them
Files:
src/platform/workflow/persistence/base/draftCache.tssrc/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/persistence/stores/workflowDraftStore.tssrc/platform/workflow/management/stores/workflowStore.test.tssrc/platform/workflow/persistence/composables/useWorkflowPersistence.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/platform/workflow/persistence/base/draftCache.tssrc/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/persistence/stores/workflowDraftStore.tssrc/platform/workflow/management/stores/workflowStore.test.tssrc/platform/workflow/persistence/composables/useWorkflowPersistence.ts
src/**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,vue}: Usereffor reactive state,computed()for derived values, andwatch/watchEffectfor side effects in Composition API
Avoid usingrefwithwatchif acomputedwould suffice - minimize refs and derived state
Useprovide/injectfor dependency injection only when simpler alternatives (Store or shared composable) won't work
Leverage VueUse functions for performance-enhancing composables
Use VueUse function for useI18n in composition API for string literals
Files:
src/platform/workflow/persistence/base/draftCache.tssrc/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/persistence/stores/workflowDraftStore.tssrc/platform/workflow/management/stores/workflowStore.test.tssrc/platform/workflow/persistence/composables/useWorkflowPersistence.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Keep functions short and functional
Minimize nesting (if statements, for loops, etc.)
Use function declarations instead of function expressions when possible
Files:
src/platform/workflow/persistence/base/draftCache.tssrc/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/persistence/stores/workflowDraftStore.tssrc/platform/workflow/management/stores/workflowStore.test.tssrc/platform/workflow/persistence/composables/useWorkflowPersistence.ts
src/**/stores/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/stores/**/*.{ts,tsx}: Maintain clear public interfaces and restrict extension access in stores
Use TypeScript for type safety in state management stores
Files:
src/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/persistence/stores/workflowDraftStore.tssrc/platform/workflow/management/stores/workflowStore.test.ts
src/**/{services,composables}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/{services,composables}/**/*.{ts,tsx}: Useapi.apiURL()for backend endpoints instead of constructing URLs directly
Useapi.fileURL()for static file access instead of constructing URLs directly
Files:
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts
src/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts
src/**/{components,composables}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Use vue-i18n for ALL user-facing strings by adding them to
src/locales/en/main.json
Files:
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts
🧠 Learnings (42)
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Write tests for all changes, especially bug fixes to catch future regressions
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tsbrowser_tests/tests/colorPalette.spec.tssrc/platform/workflow/management/stores/workflowStore.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Aim for behavioral coverage of critical and new features in unit tests
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tssrc/platform/workflow/management/stores/workflowStore.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to +(tests-ui|src|browser_tests)/**/*.+(test.ts|spec.ts) : Be parsimonious in testing - do not write redundant tests
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tsbrowser_tests/tests/colorPalette.spec.tssrc/platform/workflow/management/stores/workflowStore.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test user workflows in browser tests
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tsbrowser_tests/tests/colorPalette.spec.tssrc/platform/workflow/management/stores/workflowStore.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to +(tests-ui|src|browser_tests)/**/*.+(test.ts|spec.ts) : Do not write change detector tests - avoid tests that only assert default values
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tsbrowser_tests/tests/colorPalette.spec.tssrc/platform/workflow/management/stores/workflowStore.test.ts
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Do not write tests dependent on non-behavioral features like utility classes or styles
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tsbrowser_tests/tests/colorPalette.spec.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{js,ts,jsx,tsx} : When adding features, always write vitest unit tests using cursor rules in @.cursor
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.ts
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Do not write tests that just test the mocks - ensure tests fail when code behaves unexpectedly
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Keep module mocks contained - do not use global mutable state within test files; use `vi.hoisted()` if necessary
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tssrc/platform/workflow/management/stores/workflowStore.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Unit and component tests belong in `tests-ui/` or `src/**/*.test.ts` using Vitest
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.ts
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Leverage Vitest's utilities for mocking where possible
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : Use provided test helpers `createTestSubgraph` and `createTestSubgraphNode` from `./fixtures/subgraphHelpers` for consistent subgraph test setup
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Prefer the use of `test.extend` over loose variables; import `test as baseTest` from `vitest`
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.ts
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tsbrowser_tests/tests/colorPalette.spec.tssrc/platform/workflow/persistence/base/draftCache.tssrc/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/persistence/stores/workflowDraftStore.tssrc/platform/workflow/management/stores/workflowStore.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.tssrc/platform/workflow/persistence/composables/useWorkflowPersistence.ts
📚 Learning: 2025-12-13T11:03:11.264Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tsbrowser_tests/tests/colorPalette.spec.tssrc/platform/workflow/persistence/base/draftCache.tssrc/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/persistence/stores/workflowDraftStore.tssrc/platform/workflow/management/stores/workflowStore.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.tssrc/platform/workflow/persistence/composables/useWorkflowPersistence.ts
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tsbrowser_tests/tests/colorPalette.spec.tssrc/platform/workflow/persistence/base/draftCache.tssrc/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/persistence/stores/workflowDraftStore.tssrc/platform/workflow/management/stores/workflowStore.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.tssrc/platform/workflow/persistence/composables/useWorkflowPersistence.ts
📚 Learning: 2025-12-30T22:22:33.836Z
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tsbrowser_tests/tests/colorPalette.spec.tssrc/platform/workflow/persistence/base/draftCache.tssrc/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/persistence/stores/workflowDraftStore.tssrc/platform/workflow/management/stores/workflowStore.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.tssrc/platform/workflow/persistence/composables/useWorkflowPersistence.ts
📚 Learning: 2025-12-10T03:09:13.807Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7303
File: src/components/topbar/CurrentUserPopover.test.ts:199-205
Timestamp: 2025-12-10T03:09:13.807Z
Learning: In test files, prefer selecting or asserting on accessible properties (text content, aria-label, role, accessible name) over data-testid attributes. This ensures tests validate actual user-facing behavior and accessibility, reducing reliance on implementation details like test IDs.
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tssrc/platform/workflow/management/stores/workflowStore.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts
📚 Learning: 2025-12-30T01:31:04.927Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7797
File: tests-ui/tests/lib/litegraph/src/widgets/ComboWidget.test.ts:648-648
Timestamp: 2025-12-30T01:31:04.927Z
Learning: In Vitest v4, when mocking functions that may be called as constructors (using new), the mock implementation must use function() or class syntax rather than an arrow function. Arrow mocks can cause '<anonymous> is not a constructor' errors. This is a breaking change from Vitest v3 where mocks could use an arrow function. Apply this guideline to test files that mock constructor-like calls (e.g., in tests under tests-ui, such as ComboWidget.test.ts) and ensure mock implementations are defined with function() { ... } or class { ... } to preserve constructor behavior.
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tssrc/platform/workflow/management/stores/workflowStore.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts
📚 Learning: 2026-01-08T02:40:15.482Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7894
File: src/renderer/extensions/vueNodes/widgets/components/WidgetToggleSwitch.test.ts:11-14
Timestamp: 2026-01-08T02:40:15.482Z
Learning: In TypeScript test files (e.g., any test under src), avoid duplicating interface/type definitions. Import real type definitions from the component modules under test and reference them directly, so there is a single source of truth and to prevent type drift. This improves maintainability and consistency across tests.
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tssrc/platform/workflow/management/stores/workflowStore.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts
📚 Learning: 2026-01-09T02:07:54.558Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7898
File: src/composables/usePaste.test.ts:248-248
Timestamp: 2026-01-09T02:07:54.558Z
Learning: In test files (e.g., any .test.ts or .test.tsx under src/...), when you create mock objects that partially implement an interface (such as LGraphNode), prefer casting with as Partial<InterfaceType> as InterfaceType rather than as any or as unknown as InterfaceType. This makes the incomplete implementation explicit while preserving type safety, improving readability and maintainability of tests.
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tssrc/platform/workflow/management/stores/workflowStore.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tsbrowser_tests/tests/colorPalette.spec.tssrc/platform/workflow/persistence/base/draftCache.tssrc/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/persistence/stores/workflowDraftStore.tssrc/platform/workflow/management/stores/workflowStore.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.tssrc/platform/workflow/persistence/composables/useWorkflowPersistence.ts
📚 Learning: 2025-12-22T21:36:38.905Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: tests-ui/tests/platform/cloud/subscription/components/SubscriptionPanel.test.ts:189-194
Timestamp: 2025-12-22T21:36:38.905Z
Learning: In tests-ui feature tests, do not stub primitive UI components or customized primitives (e.g., Button). Instead, import and register the real components in the test setup so tests reflect production behavior and component API usage. This improves test reliability and fidelity. If a test must avoid rendering the full component, justify the approach and consider higher-level mocks or integration tests rather than replacing the primitive itself.
Applied to files:
tests-ui/tests/platform/workflow/persistence/draftCache.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts
📚 Learning: 2025-12-13T05:54:35.779Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7415
File: browser_tests/tests/mobileBaseline.spec.ts:17-22
Timestamp: 2025-12-13T05:54:35.779Z
Learning: In browser_tests tests for the Comfy-Org/ComfyUI_frontend repository, the `comfyPage.loadWorkflow()` method already handles all necessary synchronization and waiting. No additional `await comfyPage.nextFrame()` call is needed before taking screenshots after loading a workflow.
Applied to files:
browser_tests/tests/colorPalette.spec.tssrc/platform/workflow/management/stores/workflowStore.test.ts
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to browser_tests/**/*.spec.ts : Do not use waitForTimeout in Playwright tests - use Locator actions and retrying assertions instead
Applied to files:
browser_tests/tests/colorPalette.spec.ts
📚 Learning: 2025-12-12T23:02:37.473Z
Learnt from: Myestery
Repo: Comfy-Org/ComfyUI_frontend PR: 7422
File: .github/workflows/pr-update-playwright-expectations.yaml:131-135
Timestamp: 2025-12-12T23:02:37.473Z
Learning: In the `.github/workflows/pr-update-playwright-expectations.yaml` workflow in the Comfy-Org/ComfyUI_frontend repository, the snapshot update process is intentionally scoped to only add and update snapshot images. Deletions of snapshot files are handled explicitly outside this workflow and should not be suggested as part of this automation.
Applied to files:
browser_tests/tests/colorPalette.spec.tssrc/platform/workflow/management/stores/workflowStore.test.tstests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to browser_tests/**/*.spec.ts : Follow Playwright best practices for E2E testing
Applied to files:
browser_tests/tests/colorPalette.spec.tssrc/platform/workflow/management/stores/workflowStore.test.ts
📚 Learning: 2025-12-13T05:34:15.488Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7415
File: browser_tests/tests/mobileBaseline.spec.ts:7-15
Timestamp: 2025-12-13T05:34:15.488Z
Learning: In Playwright tests for the ComfyUI frontend, the toPass() assertion uses incremental backoff during retries. When a test may involve async operations, increasing the timeout (e.g., to 5000 ms) can be sufficient instead of aggressively extending timeouts. Apply this understanding to tests under browser_tests/tests/; if not resolved, review the toPass() backoff behavior and ensure timeouts align with expected async completion without masking issues.
Applied to files:
browser_tests/tests/colorPalette.spec.ts
📚 Learning: 2026-01-12T17:39:27.738Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7906
File: src/components/sidebar/tabs/AssetsSidebarTab.vue:545-552
Timestamp: 2026-01-12T17:39:27.738Z
Learning: In Vue/TypeScript files (src/**/*.{ts,tsx,vue}), prefer if/else statements over ternary operators when performing side effects or actions (e.g., mutating state, calling methods with side effects). Ternaries should be reserved for computing and returning values.
Applied to files:
src/platform/workflow/persistence/base/draftCache.tssrc/platform/workflow/management/stores/workflowStore.tssrc/platform/workflow/persistence/stores/workflowDraftStore.tssrc/platform/workflow/management/stores/workflowStore.test.tssrc/platform/workflow/persistence/composables/useWorkflowPersistence.ts
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/stores/**/*.ts : Name Pinia stores as `*Store.ts` (e.g., `nodeStore.ts`)
Applied to files:
src/platform/workflow/persistence/stores/workflowDraftStore.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/stores/**/*.{ts,tsx} : Maintain clear public interfaces and restrict extension access in stores
Applied to files:
src/platform/workflow/persistence/stores/workflowDraftStore.ts
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to +(tests-ui|src|browser_tests)/**/*.+(test.ts|spec.ts) : Don't Mock What You Don't Own
Applied to files:
tests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.{ts,vue} : Use `ref` for reactive state, `computed()` for derived values, and `watch`/`watchEffect` for side effects in Composition API
Applied to files:
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.{ts,vue} : Avoid using `ref` with `watch` if a `computed` would suffice - minimize refs and derived state
Applied to files:
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.{ts,vue} : Leverage VueUse functions for performance-enhancing composables
Applied to files:
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Utilize ref and reactive for reactive state
Applied to files:
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use lifecycle hooks: onMounted, onUpdated in Vue 3 Composition API
Applied to files:
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use ref/reactive for state management in Vue 3 Composition API
Applied to files:
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Use watch and watchEffect for side effects
Applied to files:
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.{vue,ts} : Leverage VueUse functions for performance-enhancing styles
Applied to files:
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.{vue,ts,js} : Use useIntersectionObserver for visibility detection instead of custom scroll handlers
Applied to files:
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Implement lifecycle hooks with onMounted, onUpdated, etc.
Applied to files:
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts
🧬 Code graph analysis (5)
browser_tests/tests/colorPalette.spec.ts (1)
src/stores/queueStore.ts (1)
workflow(315-317)
src/platform/workflow/management/stores/workflowStore.ts (2)
src/platform/workflow/validation/schemas/workflowSchema.ts (1)
ComfyWorkflowJSON(459-461)src/scripts/changeTracker.ts (1)
ChangeTracker(28-446)
src/platform/workflow/persistence/stores/workflowDraftStore.ts (1)
src/platform/workflow/persistence/base/draftCache.ts (9)
WorkflowDraftSnapshot(1-6)mostRecentDraftPath(76-77)DraftCacheState(8-11)createDraftCacheState(15-18)upsertDraft(26-44)MAX_DRAFTS(13-13)removeDraft(46-55)moveDraft(57-74)touchEntry(20-24)
src/platform/workflow/management/stores/workflowStore.test.ts (2)
src/platform/workflow/persistence/stores/workflowDraftStore.ts (1)
useWorkflowDraftStore(30-154)src/scripts/defaultGraph.ts (2)
defaultGraph(3-142)defaultGraphJSON(144-144)
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts (4)
src/platform/workflow/persistence/stores/workflowDraftStore.ts (1)
useWorkflowDraftStore(30-154)src/scripts/api.ts (1)
api(1336-1336)src/platform/workflow/management/stores/workflowStore.ts (2)
key(56-58)ComfyWorkflow(35-167)src/scripts/utils.ts (1)
getStorageValue(96-102)
🔇 Additional comments (33)
browser_tests/tests/colorPalette.spec.ts (1)
235-258: Improved synchronization approach for workflow persistence validation.The change from a fixed delay to
waitForFunctionpolling localStorage is a reasonable approach for this test case. The defensive validation (checkingparsed.nodesexists and is an array before iteration) properly handles edge cases.However, note that per coding guidelines,
waitForTimeoutshould be avoided in favor of Locator actions and retrying assertions. WhilewaitForFunctionis better than a fixed timeout, consider whether a retrying assertion pattern could work here in the future.tests-ui/tests/platform/workflow/persistence/draftCache.test.ts (1)
1-77: Good behavioral coverage for draft cache operations.The test suite properly covers the core behaviors of the draft cache module:
touchEntrymoves existing entries to end and adds new entriesupsertDraftenforces LRU eviction when exceedingMAX_DRAFTSremoveDraftclears both the draft entry and ordermoveDraftrenames entries and updates ordermostRecentDraftPathreturns the last entry or nullThe tests are parsimonious and focus on behavioral outcomes rather than implementation details.
tests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts (4)
35-61: Well-structured mock setup usingvi.hoisted().The use of
vi.hoisted()for maintaining mock state is the correct pattern per coding guidelines. This ensures the mocks are properly hoisted and avoids issues with global mutable state.The mock API surface is comprehensive, covering the necessary methods for workflow persistence testing.
112-144: Thorough multi-workflow persistence test.This test properly validates that the persistence system:
- Saves drafts for multiple workflows independently
- Preserves the correct workflow data in each draft
- Maintains the
isTemporaryflagThe use of fake timers with
advanceTimersByTimeAsync(800)correctly simulates the debounced persistence behavior.
146-167: Correct LRU eviction boundary test.The test validates that when 33 drafts are created:
- Only 32 remain (respecting
MAX_DRAFTS)- The oldest draft (
Draft0) is evicted- The newest draft (
Draft32) is retainedThe time advancement between saves ensures proper recency ordering.
169-196: Validates temporary workflow restoration from drafts.This test covers a critical user scenario: restoring unsaved temporary workflows after a page refresh. It properly verifies that:
- The workflow is recreated from the cached draft
- The
isTemporaryflag is preserved- The workflow is included in
openWorkflowssrc/platform/workflow/management/stores/workflowStore.test.ts (4)
14-14: Import added for draft store integration.The import of
useWorkflowDraftStoreis correctly added to support the new draft-related test cases.
65-82: Proper test isolation with draft store reset.The addition of
localStorage.clear(),sessionStorage.clear(), anduseWorkflowDraftStore().reset()ensures each test starts with a clean slate, preventing cross-test contamination for draft-related functionality.
242-268: Validates draft precedence over remote content.The test correctly verifies that when a local draft exists, it takes precedence over the remote server content. The workflow is properly marked as modified (
isModified: true) when loaded from a draft.One minor observation: The
draftGraphobject spreadsdefaultGraph.nodesinto a new array, which is fine for this test since the comparison usestoEqual(deep equality).
270-294: Validates stale draft handling.This test covers an important edge case: when the server version is newer than the local draft, the draft should be discarded to avoid overwriting more recent changes. The test correctly:
- Creates a draft with a timestamp
- Sets
lastModifiedto be 1 second later than the draft- Verifies the draft is removed and workflow is not marked as modified
src/platform/workflow/management/stores/workflowStore.ts (6)
16-16: Import added for draft store integration.The import is correctly placed with other persistence-related imports.
87-128: Draft integration in workflow loading is well-implemented.The
load()method now correctly:
- Retrieves drafts when not force-loading
- Discards stale drafts (when
draft.updatedAt < this.lastModified)- Handles JSON parse errors gracefully with a warning and draft removal
- Applies valid drafts by setting
activeState,content, and marking as modified- Updates LRU order via
markDraftUsed()The logic flow is sound: draft validity is checked before the main load, and draft application happens after the original content is loaded, ensuring
originalContentremains the server version whilecontentandactiveStatereflect the draft.
135-145: Draft cleanup after save is correctly placed.The draft is removed after the save operation succeeds, ensuring that:
- If save fails, the draft is preserved as a backup
- Successful saves clear the draft to avoid stale data on next load
152-158: Verify draft handling in saveAs.The implementation removes the draft at the new path (
path), which is correct to ensure the newly saved file doesn't have a stale draft. However, consider whether the draft atthis.path(the original workflow's path) should also be removed or retained.If the user saves a modified workflow to a new file, the original workflow remains modified but its draft may now be orphaned (the active state was already captured in the new file).
Is it intentional that the original workflow's draft is preserved when using saveAs? If the original workflow should be considered "clean" after saveAs (since changes were saved elsewhere), consider removing its draft as well.
590-620: Draft movement correctly integrated into rename flow.The draft is moved after the workflow rename succeeds, using
moveDraft(oldPath, newPath, workflow.key). This maintains draft continuity when a workflow is renamed.The placement after the try-finally block ensures the draft is only moved if the rename operation completes successfully.
622-636: Draft removal on workflow deletion is correct.The draft is removed after the workflow is successfully deleted, preventing orphaned drafts in storage.
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts (5)
52-103: LGTM - Draft persistence flow is well-structured.The deduplication via
lastSavedJsonByPathprevents redundant writes, and the error handling correctly aborts persistence if draft saving fails. The legacy localStorage/sessionStorage writes provide backward compatibility.
105-115: LGTM - Clean delegation to draft store.The path construction correctly uses
ComfyWorkflow.basePathprefix, and the fallback logic appropriately uses the latest draft only when no specific workflow was previously active.
173-189: LGTM - Path filtering aligns with draft persistence scope.The filter ensures only workflows with the correct base path are persisted, which correctly constrains the restoration scope to draft-managed workflows.
211-225: Edge case: Non-temporary workflows are not restored from drafts.The loop skips drafts where
!draft?.isTemporary, meaning saved workflows won't be recreated from drafts if they're missing from the store. This is likely intentional (saved workflows load from server), but verify this aligns with expected behavior when saved workflows are deleted server-side.
206-231: ThecreateTemporarymethod signature correctly accepts an optionalworkflowDataparameter. Both calls in the code—with and without the parameter—are valid and properly handled by the implementation.src/platform/workflow/persistence/stores/workflowDraftStore.ts (7)
1-19: LGTM - Imports follow repository conventions.Type imports are correctly separated, and renamed imports (
moveDraftEntry,removeDraftEntry) avoid collision with store methods.
30-45: LGTM - Store setup correctly uses VueUse storage and computed.The
useStorageintegration provides automatic localStorage sync, and thecurrentState/updateStatepattern cleanly bridges reactive state with pure cache functions.
47-65: Single retry on quota exceeded is pragmatic but limited.The quota handling removes the oldest draft and retries once. If the retry also fails (e.g., single large workflow exceeds available space), the error propagates. Consider whether logging a warning about storage constraints would help users debug persistent save failures.
75-78: LGTM - Efficient touch-only operation.Directly updating
storedOrder.valuerather than callingupdateStateis appropriate here since only the order changes, avoiding unnecessary drafts object reassignment.
99-109: LGTM - Clean draft loading with automatic cleanup.The pattern of removing corrupted drafts via the
onFailurecallback ensures invalid entries don't persist and cause repeated failures.
111-141: LGTM - Multi-stage loading cascade is well-structured.The priority order (preferred path → latest draft → session → local storage) provides robust fallback behavior, and the
initialClientId ?? clientIdpattern aligns withgetStorageValueinutils.ts.
143-154: LGTM - Clean public interface with good encapsulation.The store exposes only the necessary methods while keeping
storedDraftsandstoredOrderinternal. Theresetmethod is useful for testing scenarios.src/platform/workflow/persistence/base/draftCache.ts (5)
1-11: LGTM - Clean type definitions.The
WorkflowDraftSnapshotinterface captures all necessary metadata for draft management, andDraftCacheStateprovides a clear shape for the cache structure.
20-24: LGTM - Correct LRU touch semantics.The function correctly moves the touched path to the end of the order array, maintaining recency ordering for the LRU cache.
46-55: LGTM - Correct removal with early return optimization.The early return for non-existent paths avoids unnecessary object creation, and the implementation maintains immutability by creating new objects.
57-74: LGTM - Move operation correctly handles order updates.The implementation properly removes both old and new paths from the order before touching the new path, preventing duplicate entries and handling edge cases where the new path might have previously existed.
76-77: LGTM - Simple and correct most-recent accessor.Returns the last entry in the order array, which is correctly maintained as the most recently accessed draft by
touchEntry.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts
Outdated
Show resolved
Hide resolved
christian-byrne
added
the
preview-cpu
Yourz
force-pushed
the
workflow-persistence
branch
from
6b42c0f to
0339e96
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts (1)
50-103: Don’t mark drafts as “saved” before a successful write.
lastSavedJsonByPathis updated before the draft/local/session storage writes. IfsaveDraftor storage throws, subsequent saves of the same JSON are skipped and the draft may never persist. Move the assignment to after successful storage writes (or reset it on failure).🔧 Proposed fix
- if (workflowJson === lastSavedJsonByPath.value[workflowPath]) return - lastSavedJsonByPath.value[workflowPath] = workflowJson + if (workflowJson === lastSavedJsonByPath.value[workflowPath]) return @@ - try { + try { workflowDraftStore.saveDraft(activeWorkflow.path, { data: workflowJson, updatedAt: Date.now(), name: activeWorkflow.key, isTemporary: activeWorkflow.isTemporary }) } catch (error) { console.error('Failed to save draft', error) // If draft store fails, don't continue saving to storage return } try { localStorage.setItem('workflow', workflowJson) if (api.clientId) { sessionStorage.setItem(`workflow:${api.clientId}`, workflowJson) } } catch (error) { @@ throw error } + + lastSavedJsonByPath.value[workflowPath] = workflowJson
🤖 Fix all issues with AI agents
In `@src/platform/workflow/persistence/composables/useWorkflowPersistence.ts`:
- Around line 156-159: The comment above the direct call to
persistCurrentWorkflow() is misleading because it claims a debounced save is
used while the code calls the function immediately; either replace the call with
the debounced variant (e.g., useDebouncedPersist or
debouncedPersistCurrentWorkflow if such a wrapper exists/was intended) to
actually debounce saves, or update/remove the comment to accurately reflect the
direct save behavior—change the comment and ensure the code path around
persistCurrentWorkflow() (and any activeWorkflow change handling) matches the
chosen behavior.
- Around line 211-224: When restoring storedWorkflows, currently failed JSON
parses leave the corrupted draft in storage; update the catch block after
JSON.parse(draft.data) to remove the bad draft (use
workflowDraftStore.removeDraft or the equivalent delete method) after calling
workflowStore.createTemporary(draft.name) so the draft won't keep failing on
subsequent restores; reference storedWorkflows, workflowStore.getWorkflowByPath,
workflowDraftStore.getDraft and workflowStore.createTemporary to locate the
logic and ensure the removal runs only on parse failure.
♻️ Duplicate comments (1)
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts (1)
191-197: Validate parsed storage values before use.The
JSON.parseresults are cast directly tostring[]/number. If storage is corrupted, restoration logic can break. Add runtime validation and fall back to safe defaults.As per coding guidelines, validate trusted sources before processing data.🔧 Proposed fix
- const storedWorkflows = JSON.parse( - getStorageValue('Comfy.OpenWorkflowsPaths') || '[]' - ) as string[] - const storedActiveIndex = JSON.parse( - getStorageValue('Comfy.ActiveWorkflowIndex') || '-1' - ) as number + const parsedWorkflows = JSON.parse( + getStorageValue('Comfy.OpenWorkflowsPaths') || '[]' + ) + const storedWorkflows = Array.isArray(parsedWorkflows) + ? parsedWorkflows.filter((path) => typeof path === 'string') + : [] + const parsedIndex = JSON.parse( + getStorageValue('Comfy.ActiveWorkflowIndex') || '-1' + ) + const storedActiveIndex = + typeof parsedIndex === 'number' && Number.isFinite(parsedIndex) + ? parsedIndex + : -1
Yourz
force-pushed
the
workflow-persistence
branch
from
0339e96 to
a7d4579
Compare
🔧 Auto-fixes AppliedThis PR has been automatically updated to fix linting and formatting issues.
Changes made:
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@browser_tests/tests/colorPalette.spec.ts`:
- Around line 235-255: The current waitForFunction only validates and you
re-read localStorage again; update comfyPage.page.waitForFunction so its
predicate returns the parsed workflow object (instead of true/false) after
successfully JSON.parse-ing localStorage.getItem('workflow'), then capture that
returned value (the parsed workflow) from the waitForFunction call and use it
directly (replace the subsequent page.evaluate and JSON.parse). Keep existing
error handling in the predicate (try/catch) and ensure the returned value is the
parsed object so parsed.nodes checks and the for (const node of parsed.nodes)
loop operate on that returned object.
In
`@src/platform/workflow/persistence/composables/useWorkflowPersistence.test.ts`:
- Around line 164-175: The test uses "as Record<string, any>" for drafts—replace
the any with a concrete typed shape (e.g., define a DraftEntry type { data:
string; isTemporary: boolean } and parse into Record<string, DraftEntry>) or
parse into unknown and validate with a simple type guard before assertions;
update the variable "drafts" assignment in useWorkflowPersistence.test.ts and
apply the same pattern to the other parse/assert block referenced (lines
~191-193) so subsequent JSON.parse(drafts['workflows/DraftA.json'].data) and
isTemporary checks are type-safe.
In `@src/platform/workflow/persistence/stores/draftCache.test.ts`:
- Around line 14-19: createSnapshot uses Date.now(), which makes tests
non-deterministic; change createSnapshot (returning WorkflowDraftSnapshot) to
use a deterministic timestamp by either accepting an optional timestamp
parameter or using a fixed constant (e.g., FIXED_TEST_TIME) or an incrementing
counter, and update tests to pass/expect that deterministic value so
snapshot.updatedAt is stable across runs; locate the createSnapshot helper in
draftCache.test.ts and adjust its implementation and any test assertions
accordingly.
In `@src/platform/workflow/persistence/stores/workflowDraftStore.ts`:
- Around line 47-65: In saveDraft, when a QuotaExceededError occurs and
currentState().order is empty the catch silently swallows the error and the
draft is lost; modify the catch in saveDraft to rethrow the error if there is
nothing to evict (i.e. state.order.length === 0) so callers can surface a UI
error, otherwise keep the existing eviction logic that calls removeDraftEntry
and retries upsertDraft using currentState(), ensuring you still throw the
original error when eviction cannot free space.
♻️ Duplicate comments (1)
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts (1)
199-207: Harden storage parsing beyond Array.isArray / typeof number.Storage can be corrupted; this still allows non-string entries and
NaN/Infinity. Consider validating string items and usingNumber.isFinite.♻️ Suggested tightening
- const storedWorkflows = Array.isArray(parsedWorkflows) ? parsedWorkflows : [] + const storedWorkflows = Array.isArray(parsedWorkflows) + ? parsedWorkflows.filter((entry): entry is string => typeof entry === 'string') + : [] const parsedIndex = JSON.parse( getStorageValue('Comfy.ActiveWorkflowIndex') || '-1' ) - const storedActiveIndex = typeof parsedIndex === 'number' ? parsedIndex : -1 + const storedActiveIndex = + typeof parsedIndex === 'number' && Number.isFinite(parsedIndex) + ? parsedIndex + : -1
src/platform/workflow/persistence/composables/useWorkflowPersistence.test.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/platform/workflow/persistence/stores/draftCache.test.ts`:
- Around line 14-19: Change the helper createSnapshot from a const arrow
function to a function declaration named createSnapshot that returns a
WorkflowDraftSnapshot; keep the same parameters (name: string), body (returns {
data: JSON.stringify({ name }), updatedAt: Date.now(), name, isTemporary: true
}) and type semantics so it still produces the same snapshot object and hoists
like other pure helpers.
♻️ Duplicate comments (1)
src/platform/workflow/persistence/composables/useWorkflowPersistence.ts (1)
200-208: Harden parsing of stored tab state (avoid startup crash on corrupted JSON).
JSON.parsecan throw; also validate element types and finite index values before use.🔧 Proposed fix
- const parsedWorkflows = JSON.parse( - getStorageValue('Comfy.OpenWorkflowsPaths') || '[]' - ) - const storedWorkflows = Array.isArray(parsedWorkflows) ? parsedWorkflows : [] - const parsedIndex = JSON.parse( - getStorageValue('Comfy.ActiveWorkflowIndex') || '-1' - ) - const storedActiveIndex = typeof parsedIndex === 'number' ? parsedIndex : -1 + let storedWorkflows: string[] = [] + let storedActiveIndex = -1 + try { + const parsedWorkflows = JSON.parse( + getStorageValue('Comfy.OpenWorkflowsPaths') || '[]' + ) + storedWorkflows = + Array.isArray(parsedWorkflows) && + parsedWorkflows.every((path) => typeof path === 'string') + ? parsedWorkflows + : [] + const parsedIndex = JSON.parse( + getStorageValue('Comfy.ActiveWorkflowIndex') || '-1' + ) + storedActiveIndex = + typeof parsedIndex === 'number' && Number.isFinite(parsedIndex) + ? parsedIndex + : -1 + } catch (err) { + console.warn('Failed to parse workflow tab state', err) + }As per coding guidelines, implement proper error handling for untrusted storage data.
Yourz
force-pushed
the
workflow-persistence
branch
from
c96ecd8 to
99452d7
Compare
Yourz
force-pushed
the
workflow-persistence
branch
from
99452d7 to
24514f7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/platform/workflow/core/services/workflowService.ts`:
- Around line 296-306: Wrap the call to workflowDraftStore.saveDraft inside a
try/catch so failures (quota/serialization errors) don’t abort
beforeLoadNewGraph and block the tab switch; specifically, in the block guarded
by settingStore.get('Comfy.Workflow.Persist') and when activeWorkflow.path and
activeWorkflow.activeState exist, catch any error from
workflowDraftStore.saveDraft(activeWorkflow.path, {...}) and handle it by
logging and showing a user toast/notification (e.g., toast.error or
notificationService) while allowing execution to continue.
♻️ Duplicate comments (1)
src/platform/workflow/persistence/composables/useWorkflowPersistence.test.ts (1)
191-193: Removeanyfrom drafts parsing.
Tests should avoidany; use a concrete draft entry type instead.♻️ Suggested fix
const drafts = JSON.parse( localStorage.getItem('Comfy.Workflow.Drafts') ?? '{}' - ) as Record<string, any> + ) as Record<string, { data: string; isTemporary: boolean }>
|
@pythongosssss I update the code for coderabbit review. PTAL |
4 participants
Summary
Keep all drafts in localStorage, mirroring the logic from VSCode.
Fix a bug where newly created blank workflow tabs would incorrectly restore as defaultGraph instead of blankGraph after page refresh.
Resolves Comfy-Org/desktop#910, Resolves #4057, Fixes #3665
Changes
What
restoreWorkflowTabsStateto parse and pass workflow data from drafts when recreating temporary workflowsshould not serialize color adjustments in workflowto wait for workflow persistence before assertionsBreaking
Dependencies
Review Focus