feat: add polling fallback for stale asset downloads#7926
Conversation
dosubot
bot
added
the
size:L
📝 WalkthroughWalkthroughAdds timestamp tracking to AssetDownload, makes assetId optional, introduces polling reconciliation using a new taskService to finalize stale downloads, updates stories and schemas, and adds comprehensive store tests plus testing docs and AGENTS references. Changes
Sequence Diagram(s)sequenceDiagram
participant WS as WebSocket / API events
participant Store as assetDownloadStore
participant Task as taskService (HTTP)
Note over WS,Store: Runtime asset_download WebSocket events
WS->>Store: asset_download (may omit asset_id)
Store->>Store: create/update AssetDownload (set lastUpdate)
alt event is terminal (completed/failed)
Store->>Store: move entry to finishedDownloads (record timestamp)
else non-terminal
Store->>Store: keep/update in activeDownloads
end
Note over Store,Task: Polling reconciles stale active downloads
Store->>Task: getTask(taskId) [pollStaleDownloads]
Task-->>Store: TaskResponse (status/payload)
alt TaskResponse indicates terminal and Store lacks terminal event
Store->>Store: synthesize terminal asset_download event -> move to finished
end
Possibly related PRs
✨ 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 |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/10/2026, 12:17:18 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results✅ All tests passed! ⏰ Completed at: 01/10/2026, 12:22:36 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.24 MB (baseline 3.24 MB) • 🔴 +2.87 kBMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 1.05 MB (baseline 1.05 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.63 kB (baseline 6.63 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 337 kB (baseline 337 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 198 kB (baseline 198 kB) • ⚪ 0 BReusable component library chunks
Status: 9 added / 9 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 3 added / 3 removed Utilities & Hooks — 1.41 kB (baseline 1.41 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 9.19 MB (baseline 9.19 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Status: 6 added / 6 removed Other — 4.68 MB (baseline 4.68 MB) • ⚪ 0 BBundles that do not match a named category
Status: 21 added / 21 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/stores/assetDownloadStore.test.ts:
- Around line 6-7: The module-level mutable variable eventHandler should be
hoisted for per-test control; replace the top-level declaration with
vi.hoisted(() => null) and add a getEventHandler helper to read/write it, update
dispatchDownloadEvent to use getEventHandler() instead of the bare eventHandler,
and call eventHandler.set(null) (or reset via the hoisted handle) in beforeEach
to ensure each test starts with a clean mock state.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/components/honeyToast/HoneyToast.stories.tssrc/components/toast/ProgressToastItem.stories.tssrc/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.ts
🧰 Additional context used
📓 Path-based instructions (12)
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.jsonLeverage VueUse functions for performance-enhancing utilities
Files:
src/components/honeyToast/HoneyToast.stories.tssrc/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/components/toast/ProgressToastItem.stories.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: Minimize the surface area (exported values) of each module and composable
Use es-toolkit for utility functions
Files:
src/components/honeyToast/HoneyToast.stories.tssrc/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/components/toast/ProgressToastItem.stories.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; do not mix inlinetypeimports in the same statement
Sort and group imports by plugin; runpnpm formatbefore committing
Derive component types usingvue-component-type-helpers(ComponentProps,ComponentSlots) instead of separate type files
Code should be well-designed with clear names for everything; write code that is expressive and self-documenting
Avoid redundant comments and clean up code as you go; comments should explain why, not what
Ask if there is a simpler way to implement functionality; refactor complex code to simplify it
Minimize nesting depth (e.g.,if () { ... }orfor () { ... }); watch for arrow anti-pattern
Watch out for code smells and refactor to avoid them
Never useanytype; use proper TypeScript types
Never useas anytype assertions; fix the underlying type issue instead
Indent with 2 spaces; use single quotes; no trailing semicolons; max line width 80 (per .prettierrc)
Complex type definitions used in multiple related places should be extracted and named for reuse
Implement proper error handling
Files:
src/components/honeyToast/HoneyToast.stories.tssrc/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/components/toast/ProgressToastItem.stories.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/components/honeyToast/HoneyToast.stories.tssrc/components/toast/ProgressToastItem.stories.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/components/honeyToast/HoneyToast.stories.tssrc/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/components/toast/ProgressToastItem.stories.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/components/honeyToast/HoneyToast.stories.tssrc/components/toast/ProgressToastItem.stories.ts
src/components/**/*.{vue,ts,js}
📄 CodeRabbit inference engine (src/components/CLAUDE.md)
src/components/**/*.{vue,ts,js}: Use existing VueUse composables (such as useElementHover) instead of manually managing event listeners
Use useIntersectionObserver for visibility detection instead of custom scroll handlers
Use vue-i18n for ALL UI strings
Files:
src/components/honeyToast/HoneyToast.stories.tssrc/components/toast/ProgressToastItem.stories.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Keep functions short and functional; use function declarations instead of function expressions when possible
Avoid mutable state; prefer immutability and assignment at point of declaration
Favor pure functions, especially testable ones
Files:
src/components/honeyToast/HoneyToast.stories.tssrc/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/components/toast/ProgressToastItem.stories.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/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.ts
src/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.test.ts: Write unit tests insrc/**/*.test.ts; use Vitest with happy-dom
Do not write tests that only test the mocks; ensure tests fail when code behaves unexpectedly
Use Vitest's utilities for mocking where possible; keep module mocks contained and avoid global mutable state
Use Vue Test Utils for component testing; follow advice about making components easy to test
Usevi.hoisted()in tests to allow per-test Arrange phase manipulation of mock state
Aim for behavioral coverage of critical and new features in unit tests
Files:
src/stores/assetDownloadStore.test.ts
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.ts: Do not write change detector tests that only assert default values
Do not write tests dependent on non-behavioral features like utility classes or styles
Write tests for all changes, especially bug fixes to catch future regressions
Be parsimonious in testing; do not write redundant tests (see Composable Tests pattern)
Do not mock what you don't own; avoid mocking external dependencies
Files:
src/stores/assetDownloadStore.test.ts
src/stores/**/*Store.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name Pinia store files with the
*Store.tssuffix
Files:
src/stores/assetDownloadStore.ts
🧠 Learnings (16)
📚 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:
src/components/honeyToast/HoneyToast.stories.tssrc/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/components/toast/ProgressToastItem.stories.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:
src/components/honeyToast/HoneyToast.stories.tssrc/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/components/toast/ProgressToastItem.stories.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:
src/components/honeyToast/HoneyToast.stories.tssrc/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/components/toast/ProgressToastItem.stories.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:
src/components/honeyToast/HoneyToast.stories.tssrc/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/components/toast/ProgressToastItem.stories.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:
src/components/honeyToast/HoneyToast.stories.tssrc/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/components/toast/ProgressToastItem.stories.ts
📚 Learning: 2026-01-08T21:43:58.213Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7897
File: src/stores/assetDownloadStore.ts:117-117
Timestamp: 2026-01-08T21:43:58.213Z
Learning: In src/stores/assetDownloadStore.ts, the api.addEventListener('asset_download', handleAssetDownload) call at module level is intentional and should persist for the application lifetime since it's a singleton Pinia store managing global WebSocket events.
Applied to files:
src/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.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} : Check assets/ directory for test data when writing tests
Applied to files:
src/stores/assetDownloadStore.test.ts
📚 Learning: 2026-01-09T00:50:57.103Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T00:50:57.103Z
Learning: Applies to src/**/*.test.ts : Aim for behavioral coverage of critical and new features in unit tests
Applied to files:
src/stores/assetDownloadStore.test.ts
📚 Learning: 2026-01-09T00:50:57.103Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T00:50:57.103Z
Learning: Applies to **/*.{test,spec}.ts : Write tests for all changes, especially bug fixes to catch future regressions
Applied to files:
src/stores/assetDownloadStore.test.ts
📚 Learning: 2026-01-09T00:50:57.103Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T00:50:57.103Z
Learning: Applies to src/**/*.test.ts : Use Vitest's utilities for mocking where possible; keep module mocks contained and avoid global mutable state
Applied to files:
src/stores/assetDownloadStore.test.ts
📚 Learning: 2026-01-09T00:50:57.103Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T00:50:57.103Z
Learning: Applies to src/**/*.test.ts : Use `vi.hoisted()` in tests to allow per-test Arrange phase manipulation of mock state
Applied to files:
src/stores/assetDownloadStore.test.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:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.test.ts
📚 Learning: 2026-01-06T19:17:12.355Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7746
File: src/platform/assets/composables/useUploadModelWizard.ts:207-219
Timestamp: 2026-01-06T19:17:12.355Z
Learning: In `src/platform/assets/composables/useUploadModelWizard.ts`, the `uploadModel()` function should have a concurrency guard at the beginning that checks if `isUploading.value` is already true and returns false early to prevent race conditions from multiple rapid invocations.
Applied to files:
src/stores/assetDownloadStore.ts
🧬 Code graph analysis (2)
src/stores/assetDownloadStore.test.ts (2)
src/schemas/apiSchema.ts (1)
AssetDownloadWsMessage(168-168)src/stores/assetDownloadStore.ts (1)
useAssetDownloadStore(31-153)
src/stores/assetDownloadStore.ts (1)
src/platform/assets/services/assetService.ts (1)
assetService(533-533)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: lint-and-format
- GitHub Check: test
- GitHub Check: setup
- GitHub Check: collect
🔇 Additional comments (13)
src/components/toast/ProgressToastItem.stories.ts (1)
32-32: LGTM!The addition of
lastUpdate: Date.now()correctly aligns the mock data with the updatedAssetDownloadinterface.src/components/honeyToast/HoneyToast.stories.ts (1)
20-20: LGTM!The addition of
lastUpdate: Date.now()correctly updates the mock data to match the newAssetDownloadinterface.src/stores/assetDownloadStore.test.ts (2)
147-170: LGTM! Polling test correctly validates stale download recovery.The test properly verifies the polling behavior:
- Uses
vi.advanceTimersByTimeAsync(45000)which correctly accounts for the 30s stale threshold + 15s poll interval- Verifies that
assetService.getAssetDetailsis called for stale downloads- Confirms the download transitions from active to finished with completed status
68-219: Excellent test coverage for the polling feature.The test suite comprehensively covers:
- Basic download tracking and status transitions
- Deduplication of terminal states
- Model type association for completion tracking
- Stale download polling with threshold enforcement
- Polling error handling
- Finished download cleanup
The tests follow behavioral testing principles and validate the critical paths of the new polling mechanism.
src/stores/assetDownloadStore.ts (9)
1-1: LGTM! Appropriate VueUse and service imports.The additions correctly import:
useIntervalFnfor interval-based pollingwatchfor reactive polling controlassetServicefor REST API asset fetchingAlso applies to: 3-3, 5-5
18-18: LGTM! lastUpdate field added to track stale downloads.The
lastUpdate: numberfield is essential for the polling mechanism to determine when downloads have gone stale.
28-29: LGTM! Well-chosen polling constants.The 30-second stale threshold and 15-second poll interval provide a good balance:
- Allows for transient network issues without immediate polling
- Frequent enough to recover dropped messages reasonably quickly
- Polls twice within the stale window for reliability
56-61: Excellent defensive check to prevent duplicate processing.The terminal state check ensures that completed or failed downloads are not re-processed if additional WebSocket messages arrive. This is critical for the polling fallback which synthesizes completion events.
72-72: LGTM! Timestamp updated on every download event.Setting
lastUpdate: Date.now()on each event ensures the polling logic can accurately determine staleness.
80-84: LGTM! Completion tracking updated with timestamp.The immutable update pattern correctly adds new completed downloads with timestamps while maintaining the MAX_COMPLETED_DOWNLOADS limit.
93-122: Solid polling implementation with appropriate error handling.The
pollStaleDownloadsfunction correctly:
- Filters active downloads by staleness threshold
- Skips downloads without asset IDs
- Fetches asset details from the REST API
- Synthesizes completion events via the existing
handleAssetDownloadflow- Silently handles errors (asset not ready yet)
The silent error handling on line 118-120 is appropriate here since the asset may legitimately not be ready, and the next poll will retry.
Verify that the silent error handling aligns with expected backend behavior when assets are still processing.
124-128: LGTM! Proper use of VueUse's useIntervalFn.The configuration is correct:
- Uses
voidoperator for the async function (fire-and-forget is appropriate for polling)- Sets
immediate: falseto defer polling until explicitly started- Destructures
pauseandresumefor lifecycle control
130-133: Excellent reactive polling control.The watch properly starts polling when downloads become active and stops when all downloads finish, ensuring efficient resource usage.
DrJKL
force-pushed
the
feat/stale-download-polling
branch
2 times, most recently
from
5c6452d to
f5fc449
Compare
Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019ba0f4-725f-7594-897d-aa143d1c5ce9
DrJKL
force-pushed
the
feat/stale-download-polling
branch
from
f5fc449 to
020f880
Compare
Amp-Thread-ID: https://ampcode.com/threads/T-019ba0f4-725f-7594-897d-aa143d1c5ce9 Co-authored-by: Amp <amp@ampcode.com>
DrJKL
force-pushed
the
feat/stale-download-polling
branch
from
e0d3333 to
39d664a
Compare
Amp-Thread-ID: https://ampcode.com/threads/T-019ba420-7e17-7178-a285-15cae5fc9aff Co-authored-by: Amp <amp@ampcode.com>
dosubot
bot
added
size:XL
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/platform/tasks/services/taskService.ts:
- Around line 56-61: The task response validation currently throws a generic
Error with result.error.message after zTaskResponse.safeParse; instead import
and use fromZodError to format the Zod error and include it in the thrown Error
to provide clearer, user-friendly messages without leaking schema internals (add
import of fromZodError and replace the throw new Error(...) in the safeParse
failure branch to throw a new Error(fromZodError(result.error))).
In @src/stores/assetDownloadStore.ts:
- Around line 93-124: pollStaleDownloads currently iterates activeDownloads
sequentially and awaits each taskService.getTask call one-by-one, which can
stall recovery when many items are stale; change it to collect stale entries
(using activeDownloads.value and STALE_THRESHOLD_MS), then perform the
taskService.getTask calls in parallel with bounded concurrency (e.g., batch them
or use a concurrency limiter) and for each resolved task invoke
handleAssetDownload with the same event construction and error handling; keep
the existing task.status checks and catch block but replace the for...of await
sequence with Promise.all over the batched/limited async map to improve
scalability.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
AGENTS.mddocs/testing/vitest-patterns.mdsrc/components/honeyToast/HoneyToast.stories.tssrc/components/toast/ProgressToastItem.stories.tssrc/platform/tasks/services/taskService.tssrc/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.ts
🧰 Additional context used
📓 Path-based instructions (13)
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.jsonLeverage VueUse functions for performance-enhancing utilities
Files:
src/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/platform/tasks/services/taskService.tssrc/components/honeyToast/HoneyToast.stories.tssrc/components/toast/ProgressToastItem.stories.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: Minimize the surface area (exported values) of each module and composable
Use es-toolkit for utility functions
Files:
src/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/platform/tasks/services/taskService.tssrc/components/honeyToast/HoneyToast.stories.tssrc/components/toast/ProgressToastItem.stories.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; do not mix inlinetypeimports in the same statement
Sort and group imports by plugin; runpnpm formatbefore committing
Derive component types usingvue-component-type-helpers(ComponentProps,ComponentSlots) instead of separate type files
Code should be well-designed with clear names for everything; write code that is expressive and self-documenting
Avoid redundant comments and clean up code as you go; comments should explain why, not what
Ask if there is a simpler way to implement functionality; refactor complex code to simplify it
Minimize nesting depth (e.g.,if () { ... }orfor () { ... }); watch for arrow anti-pattern
Watch out for code smells and refactor to avoid them
Never useanytype; use proper TypeScript types
Never useas anytype assertions; fix the underlying type issue instead
Indent with 2 spaces; use single quotes; no trailing semicolons; max line width 80 (per .prettierrc)
Complex type definitions used in multiple related places should be extracted and named for reuse
Implement proper error handling
Files:
src/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/platform/tasks/services/taskService.tssrc/components/honeyToast/HoneyToast.stories.tssrc/components/toast/ProgressToastItem.stories.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/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/platform/tasks/services/taskService.tssrc/components/honeyToast/HoneyToast.stories.tssrc/components/toast/ProgressToastItem.stories.ts
src/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.test.ts: Write unit tests insrc/**/*.test.ts; use Vitest with happy-dom
Do not write tests that only test the mocks; ensure tests fail when code behaves unexpectedly
Use Vitest's utilities for mocking where possible; keep module mocks contained and avoid global mutable state
Use Vue Test Utils for component testing; follow advice about making components easy to test
Usevi.hoisted()in tests to allow per-test Arrange phase manipulation of mock state
Aim for behavioral coverage of critical and new features in unit tests
Files:
src/stores/assetDownloadStore.test.ts
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.ts: Do not write change detector tests that only assert default values
Do not write tests dependent on non-behavioral features like utility classes or styles
Write tests for all changes, especially bug fixes to catch future regressions
Be parsimonious in testing; do not write redundant tests (see Composable Tests pattern)
Do not mock what you don't own; avoid mocking external dependencies
Files:
src/stores/assetDownloadStore.test.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Keep functions short and functional; use function declarations instead of function expressions when possible
Avoid mutable state; prefer immutability and assignment at point of declaration
Favor pure functions, especially testable ones
Files:
src/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/platform/tasks/services/taskService.tssrc/components/honeyToast/HoneyToast.stories.tssrc/components/toast/ProgressToastItem.stories.ts
src/stores/**/*Store.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name Pinia store files with the
*Store.tssuffix
Files:
src/stores/assetDownloadStore.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/tasks/services/taskService.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/components/honeyToast/HoneyToast.stories.tssrc/components/toast/ProgressToastItem.stories.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/components/honeyToast/HoneyToast.stories.tssrc/components/toast/ProgressToastItem.stories.ts
src/components/**/*.{vue,ts,js}
📄 CodeRabbit inference engine (src/components/CLAUDE.md)
src/components/**/*.{vue,ts,js}: Use existing VueUse composables (such as useElementHover) instead of manually managing event listeners
Use useIntersectionObserver for visibility detection instead of custom scroll handlers
Use vue-i18n for ALL UI strings
Files:
src/components/honeyToast/HoneyToast.stories.tssrc/components/toast/ProgressToastItem.stories.ts
🧠 Learnings (32)
📚 Learning: 2026-01-08T21:43:58.213Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7897
File: src/stores/assetDownloadStore.ts:117-117
Timestamp: 2026-01-08T21:43:58.213Z
Learning: In src/stores/assetDownloadStore.ts, the api.addEventListener('asset_download', handleAssetDownload) call at module level is intentional and should persist for the application lifetime since it's a singleton Pinia store managing global WebSocket events.
Applied to files:
src/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.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} : Check assets/ directory for test data when writing tests
Applied to files:
src/stores/assetDownloadStore.test.ts
📚 Learning: 2026-01-09T00:50:57.103Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T00:50:57.103Z
Learning: Applies to src/**/*.test.ts : Aim for behavioral coverage of critical and new features in unit tests
Applied to files:
src/stores/assetDownloadStore.test.ts
📚 Learning: 2026-01-09T00:50:57.103Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T00:50:57.103Z
Learning: Applies to **/*.{test,spec}.ts : Write tests for all changes, especially bug fixes to catch future regressions
Applied to files:
src/stores/assetDownloadStore.test.tsdocs/testing/vitest-patterns.md
📚 Learning: 2026-01-09T00:50:57.103Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T00:50:57.103Z
Learning: Applies to src/**/*.test.ts : Use `vi.hoisted()` in tests to allow per-test Arrange phase manipulation of mock state
Applied to files:
src/stores/assetDownloadStore.test.tsdocs/testing/vitest-patterns.md
📚 Learning: 2026-01-09T00:50:57.103Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T00:50:57.103Z
Learning: Applies to src/**/*.test.ts : Use Vitest's utilities for mocking where possible; keep module mocks contained and avoid global mutable state
Applied to files:
src/stores/assetDownloadStore.test.tsdocs/testing/vitest-patterns.mdAGENTS.md
📚 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} : Mocks should be cleanly written and easy to understand, with reusable mocks where possible
Applied to files:
src/stores/assetDownloadStore.test.ts
📚 Learning: 2026-01-09T00:50:57.103Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T00:50:57.103Z
Learning: Applies to browser_tests/**/*.spec.ts : Follow Playwright best practices as described in the official documentation
Applied to files:
src/stores/assetDownloadStore.test.tsdocs/testing/vitest-patterns.mdAGENTS.md
📚 Learning: 2026-01-09T00:50:57.103Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T00:50:57.103Z
Learning: Applies to src/**/*.test.ts : Do not write tests that only test the mocks; ensure tests fail when code behaves unexpectedly
Applied to files:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/platform/tasks/services/taskService.tssrc/components/honeyToast/HoneyToast.stories.tssrc/components/toast/ProgressToastItem.stories.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:
src/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/platform/tasks/services/taskService.tssrc/components/honeyToast/HoneyToast.stories.tssrc/components/toast/ProgressToastItem.stories.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:
src/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/platform/tasks/services/taskService.tssrc/components/honeyToast/HoneyToast.stories.tssrc/components/toast/ProgressToastItem.stories.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:
src/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/platform/tasks/services/taskService.tssrc/components/honeyToast/HoneyToast.stories.tssrc/components/toast/ProgressToastItem.stories.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:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.test.tssrc/stores/assetDownloadStore.tssrc/platform/tasks/services/taskService.tssrc/components/honeyToast/HoneyToast.stories.tssrc/components/toast/ProgressToastItem.stories.ts
📚 Learning: 2026-01-06T19:17:12.355Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7746
File: src/platform/assets/composables/useUploadModelWizard.ts:207-219
Timestamp: 2026-01-06T19:17:12.355Z
Learning: In `src/platform/assets/composables/useUploadModelWizard.ts`, the `uploadModel()` function should have a concurrency guard at the beginning that checks if `isUploading.value` is already true and returns false early to prevent race conditions from multiple rapid invocations.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2026-01-06T19:20:56.167Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7746
File: src/platform/assets/services/assetService.ts:484-491
Timestamp: 2026-01-06T19:20:56.167Z
Learning: In `src/platform/assets/services/assetService.ts`, prefer using `schema.safeParse()` over `schema.parse()` for validating API responses to avoid throwing ZodError with internal schema details; follow the existing pattern: call `safeParse()`, check `result.success`, and use `fromZodError(result.error)` to format error messages for logging while throwing user-friendly errors.
Applied to files:
src/platform/tasks/services/taskService.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} : Use `vitest` for unit testing in this project
Applied to files:
docs/testing/vitest-patterns.mdAGENTS.md
📚 Learning: 2026-01-09T00:50:57.103Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T00:50:57.103Z
Learning: Applies to src/**/*.test.ts : Write unit tests in `src/**/*.test.ts`; use Vitest with happy-dom
Applied to files:
docs/testing/vitest-patterns.mdAGENTS.md
📚 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} : Use `test` instead of `it` for defining test cases in vitest
Applied to files:
docs/testing/vitest-patterns.mdAGENTS.md
📚 Learning: 2025-12-30T01:31:11.188Z
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:11.188Z
Learning: In Vitest v4, when mocking functions that may be called as constructors (with `new` keyword), the mock implementation must use `function()` or `class` syntax instead of arrow functions. Arrow functions will cause `<anonymous> is not a constructor` errors. This is a breaking change from Vitest v3 where mocks used `mock.apply`.
Applied to files:
docs/testing/vitest-patterns.md
📚 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:
docs/testing/vitest-patterns.md
📚 Learning: 2026-01-09T00:50:57.103Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T00:50:57.103Z
Learning: Applies to src/**/*.test.ts : Use Vue Test Utils for component testing; follow advice about making components easy to test
Applied to files:
docs/testing/vitest-patterns.mdAGENTS.md
📚 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:
docs/testing/vitest-patterns.md
📚 Learning: 2026-01-09T00:50:57.103Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T00:50:57.103Z
Learning: Applies to **/*.{test,spec}.ts : Be parsimonious in testing; do not write redundant tests (see Composable Tests pattern)
Applied to files:
docs/testing/vitest-patterns.md
📚 Learning: 2026-01-09T00:50:57.103Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T00:50:57.103Z
Learning: Applies to browser_tests/**/*.spec.ts : Write E2E tests in `browser_tests/**/*.spec.ts`; use Playwright
Applied to files:
AGENTS.md
📚 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:
AGENTS.md
📚 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} : Use Playwright fixtures for browser tests
Applied to files:
AGENTS.md
📚 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} : Follow naming conventions for browser tests
Applied to files:
AGENTS.md
🧬 Code graph analysis (3)
src/stores/assetDownloadStore.test.ts (3)
src/schemas/apiSchema.ts (1)
AssetDownloadWsMessage(168-168)src/stores/assetDownloadStore.ts (1)
useAssetDownloadStore(31-155)src/platform/tasks/services/taskService.ts (2)
TaskResponse(43-43)taskService(69-69)
src/stores/assetDownloadStore.ts (1)
src/platform/tasks/services/taskService.ts (1)
taskService(69-69)
src/platform/tasks/services/taskService.ts (1)
src/scripts/api.ts (1)
api(1309-1309)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: playwright-tests-chromium-sharded (3, 8)
- GitHub Check: playwright-tests-chromium-sharded (2, 8)
- GitHub Check: playwright-tests-chromium-sharded (5, 8)
- GitHub Check: playwright-tests-chromium-sharded (4, 8)
- GitHub Check: playwright-tests-chromium-sharded (6, 8)
- GitHub Check: playwright-tests-chromium-sharded (1, 8)
- GitHub Check: playwright-tests-chromium-sharded (8, 8)
- GitHub Check: playwright-tests-chromium-sharded (7, 8)
- GitHub Check: playwright-tests (chromium-0.5x)
- GitHub Check: playwright-tests (chromium-2x)
- GitHub Check: playwright-tests (mobile-chrome)
- GitHub Check: test
🔇 Additional comments (20)
AGENTS.md (1)
164-165: LGTM! Documentation reference is helpful.The cross-reference to the new testing patterns documentation is appropriate and aligns with the new
docs/testing/vitest-patterns.mdfile added in this PR.docs/testing/vitest-patterns.md (1)
1-138: LGTM! Excellent testing documentation.This documentation provides clear, practical patterns that align with the project's testing conventions and Vitest best practices. The examples are well-chosen and directly applicable to the codebase.
src/platform/tasks/services/taskService.ts (1)
1-9: Good caveat documentation.The comment clearly documents the limitation that these schemas are specific to
task:download_filetasks. This is appropriate for the initial implementation.src/components/toast/ProgressToastItem.stories.ts (1)
23-35: LGTM! Mock data updated correctly.The addition of
lastUpdateto the mock aligns with the updatedAssetDownloadinterface. This ensures Storybook stories remain functional with the new polling feature.src/components/honeyToast/HoneyToast.stories.ts (1)
11-23: LGTM! Mock data updated correctly.The addition of
lastUpdateto the mock keeps this story file consistent with the updatedAssetDownloadinterface, matching the identical change inProgressToastItem.stories.ts.src/stores/assetDownloadStore.test.ts (8)
1-8: LGTM! Imports are properly organized.Type imports are correctly separated from value imports per the repository's lint rules. The use of
vi.hoisted()for the event handler state follows the coding guidelines for per-test mock state manipulation.
17-30: LGTM! Mocks are cleanly scoped.The mocks for
apiandtaskServiceare self-contained and capture the event handler for test manipulation. This avoids global mutable state per the testing guidelines.
32-54: LGTM! Well-designed test helpers.
createDownloadMessageanddispatchare reusable helpers that keep tests concise. The error guard indispatchprovides helpful debugging feedback if the store isn't initialized.
56-66: LGTM! Proper test isolation setup.Using
createTestingPinia({ stubActions: false })ensures real action execution while still providing test isolation. Fake timers and mock resets ensure clean state between tests.
68-114: LGTM! Good behavioral coverage for handleAssetDownload.Tests verify:
- Running downloads are tracked
- Completed downloads move to finished state
- Failed downloads move to finished state with error
- Duplicate terminal state messages are ignored
These tests validate actual store behavior rather than implementation details.
131-166: LGTM! Polling success path is well tested.The 45s advance correctly triggers the stale threshold (30s) and allows the poll interval (15s) to fire. The test verifies the integration with
taskService.getTaskand confirms the download transitions to completed state.
168-210: LGTM! Comprehensive polling edge case coverage.Tests cover:
- Failed task status propagation with error message
- No state change when backend task is still running
- Graceful handling of polling errors (download remains active)
This ensures the polling fallback is resilient to various backend states.
213-224: LGTM! clearFinishedDownloads test is straightforward.Simple behavioral test that verifies finished downloads are removed from the store.
src/stores/assetDownloadStore.ts (7)
1-7: LGTM! Imports follow repository conventions.Type imports are properly separated from value imports. Using
useIntervalFnfrom VueUse for polling aligns with the coding guidelines to leverage VueUse for utility functions.
18-18: LGTM! lastUpdate field enables staleness detection.Adding
lastUpdate: numberto the interface provides the timestamp needed to determine when a download should be polled.
28-29: LGTM! Reasonable polling constants.30-second stale threshold balances responsiveness with avoiding premature polling. 15-second interval provides recovery without overwhelming the backend.
56-61: LGTM! Terminal state guard prevents duplicate processing.The early return for completed/failed downloads ensures idempotency when WebSocket messages arrive after polling has already reconciled the state.
120-122: Empty catch block is appropriate here.The silent catch aligns with the polling fallback design—errors are expected (task not found, network hiccups) and the next poll interval will retry. No action needed.
126-135: LGTM! Clean interval lifecycle management.Using
useIntervalFnwith{ immediate: false }and awatchto start/stop based onhasActiveDownloadsensures polling only runs when needed, avoiding unnecessary network requests when idle.
145-154: LGTM! Store exposes a clean public interface.The return object maintains a minimal surface area with only the necessary computed properties and methods, keeping internal implementation (polling, event handling) private.
DrJKL
force-pushed
the
feat/stale-download-polling
branch
from
4e64de5 to
6c180e0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/stores/assetDownloadStore.ts:
- Around line 93-129: The catch in pollSingleDownload inside pollStaleDownloads
swallows all errors, hiding network/parse/unexpected failures—update
pollSingleDownload to catch and handle only known "task not ready/not found"
cases (e.g., inspect error.status or error.code) and for other errors log them
with details (using the existing logger or console.error) and either rethrow or
mark the download as failed; specifically, adjust the try/catch around
taskService.getTask and include the error object in the log, referencing
pollStaleDownloads, pollSingleDownload, taskService.getTask, and
handleAssetDownload so you only suppress genuine "not found/not ready" errors
while surface unexpected ones for debugging.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/platform/tasks/services/taskService.tssrc/stores/assetDownloadStore.ts
🧰 Additional context used
📓 Path-based instructions (8)
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.jsonLeverage VueUse functions for performance-enhancing utilities
Files:
src/platform/tasks/services/taskService.tssrc/stores/assetDownloadStore.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: Minimize the surface area (exported values) of each module and composable
Use es-toolkit for utility functions
Files:
src/platform/tasks/services/taskService.tssrc/stores/assetDownloadStore.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/tasks/services/taskService.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; do not mix inlinetypeimports in the same statement
Sort and group imports by plugin; runpnpm formatbefore committing
Derive component types usingvue-component-type-helpers(ComponentProps,ComponentSlots) instead of separate type files
Code should be well-designed with clear names for everything; write code that is expressive and self-documenting
Avoid redundant comments and clean up code as you go; comments should explain why, not what
Ask if there is a simpler way to implement functionality; refactor complex code to simplify it
Minimize nesting depth (e.g.,if () { ... }orfor () { ... }); watch for arrow anti-pattern
Watch out for code smells and refactor to avoid them
Never useanytype; use proper TypeScript types
Never useas anytype assertions; fix the underlying type issue instead
Indent with 2 spaces; use single quotes; no trailing semicolons; max line width 80 (per .prettierrc)
Complex type definitions used in multiple related places should be extracted and named for reuse
Implement proper error handling
Files:
src/platform/tasks/services/taskService.tssrc/stores/assetDownloadStore.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/platform/tasks/services/taskService.tssrc/stores/assetDownloadStore.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Keep functions short and functional; use function declarations instead of function expressions when possible
Avoid mutable state; prefer immutability and assignment at point of declaration
Favor pure functions, especially testable ones
Files:
src/platform/tasks/services/taskService.tssrc/stores/assetDownloadStore.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/stores/assetDownloadStore.ts
src/stores/**/*Store.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name Pinia store files with the
*Store.tssuffix
Files:
src/stores/assetDownloadStore.ts
🧠 Learnings (8)
📚 Learning: 2026-01-06T19:20:56.167Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7746
File: src/platform/assets/services/assetService.ts:484-491
Timestamp: 2026-01-06T19:20:56.167Z
Learning: In `src/platform/assets/services/assetService.ts`, prefer using `schema.safeParse()` over `schema.parse()` for validating API responses to avoid throwing ZodError with internal schema details; follow the existing pattern: call `safeParse()`, check `result.success`, and use `fromZodError(result.error)` to format error messages for logging while throwing user-friendly errors.
Applied to files:
src/platform/tasks/services/taskService.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:
src/platform/tasks/services/taskService.tssrc/stores/assetDownloadStore.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:
src/platform/tasks/services/taskService.tssrc/stores/assetDownloadStore.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:
src/platform/tasks/services/taskService.tssrc/stores/assetDownloadStore.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:
src/platform/tasks/services/taskService.tssrc/stores/assetDownloadStore.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:
src/platform/tasks/services/taskService.tssrc/stores/assetDownloadStore.ts
📚 Learning: 2026-01-08T21:43:58.213Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7897
File: src/stores/assetDownloadStore.ts:117-117
Timestamp: 2026-01-08T21:43:58.213Z
Learning: In src/stores/assetDownloadStore.ts, the api.addEventListener('asset_download', handleAssetDownload) call at module level is intentional and should persist for the application lifetime since it's a singleton Pinia store managing global WebSocket events.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2026-01-06T19:17:12.355Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7746
File: src/platform/assets/composables/useUploadModelWizard.ts:207-219
Timestamp: 2026-01-06T19:17:12.355Z
Learning: In `src/platform/assets/composables/useUploadModelWizard.ts`, the `uploadModel()` function should have a concurrency guard at the beginning that checks if `isUploading.value` is already true and returns false early to prevent race conditions from multiple rapid invocations.
Applied to files:
src/stores/assetDownloadStore.ts
🧬 Code graph analysis (2)
src/platform/tasks/services/taskService.ts (1)
src/scripts/api.ts (1)
api(1309-1309)
src/stores/assetDownloadStore.ts (1)
src/platform/tasks/services/taskService.ts (1)
taskService(70-70)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: playwright-tests-chromium-sharded (2, 8)
- GitHub Check: playwright-tests-chromium-sharded (7, 8)
- GitHub Check: playwright-tests-chromium-sharded (8, 8)
- GitHub Check: playwright-tests-chromium-sharded (6, 8)
- GitHub Check: playwright-tests-chromium-sharded (5, 8)
- GitHub Check: playwright-tests-chromium-sharded (4, 8)
- GitHub Check: playwright-tests-chromium-sharded (3, 8)
- GitHub Check: playwright-tests-chromium-sharded (1, 8)
- GitHub Check: playwright-tests (chromium-2x)
- GitHub Check: playwright-tests (mobile-chrome)
- GitHub Check: playwright-tests (chromium-0.5x)
- GitHub Check: test
🔇 Additional comments (5)
src/platform/tasks/services/taskService.ts (2)
46-65: LGTM: Well-structured task fetching with proper error handling.The
getTaskfunction correctly follows the established pattern:
- Uses
safeParse()instead ofparse()to avoid exposing internal Zod schema details- Applies
fromZodError()for user-friendly error messages- Handles 404 and other HTTP errors appropriately
Based on learnings from PR #7746.
30-42: Critical: Replace Zod v4 features with v3-compatible patterns.Lines 31, 38, 40, and 41 use Zod v4-only validators (
z.string().uuid()andz.string().datetime()), but this project uses Zod v3.x. These will cause runtime errors.Based on learnings, in Zod v3:
- Use
z.string()with.refine()or.regex()for UUID validation- Use
z.string()with ISO date/time string validation for datetime fields🔧 Proposed fix for Zod v3 compatibility
+// UUID regex pattern for Zod v3 +const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i + +// ISO 8601 datetime regex for Zod v3 +const ISO_DATETIME_REGEX = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d{3})?Z?$/ + const zTaskResponse = z.object({ - id: z.string().uuid(), + id: z.string().regex(UUID_REGEX, 'Invalid UUID format'), idempotency_key: z.string(), task_name: z.string(), payload: z.record(z.unknown()), status: zTaskStatus, result: zDownloadFileResult.optional(), error_message: z.string().optional(), - create_time: z.string().datetime(), - update_time: z.string().datetime(), - started_at: z.string().datetime().optional(), - completed_at: z.string().datetime().optional() + create_time: z.string().regex(ISO_DATETIME_REGEX, 'Invalid ISO datetime'), + update_time: z.string().regex(ISO_DATETIME_REGEX, 'Invalid ISO datetime'), + started_at: z.string().regex(ISO_DATETIME_REGEX, 'Invalid ISO datetime').optional(), + completed_at: z.string().regex(ISO_DATETIME_REGEX, 'Invalid ISO datetime').optional() })Based on learnings from PR #7169.
⛔ Skipped due to learnings
Learnt from: DrJKL Repo: Comfy-Org/ComfyUI_frontend PR: 7746 File: src/platform/assets/services/assetService.ts:484-491 Timestamp: 2026-01-06T19:20:56.167Z Learning: In `src/platform/assets/services/assetService.ts`, prefer using `schema.safeParse()` over `schema.parse()` for validating API responses to avoid throwing ZodError with internal schema details; follow the existing pattern: call `safeParse()`, check `result.success`, and use `fromZodError(result.error)` to format error messages for logging while throwing user-friendly errors.src/stores/assetDownloadStore.ts (3)
56-61: LGTM: Prevents duplicate processing of completed downloads.The terminal state check correctly guards against re-processing downloads that have already reached a final state, preventing unnecessary updates and potential race conditions.
131-140: LGTM: Efficient polling control with automatic start/stop.The use of
useIntervalFnwithwatchprovides clean automatic lifecycle management:
- Polling starts only when active downloads exist
- Polling stops when all downloads complete
- Avoids unnecessary polling cycles
This follows VueUse best practices for performance-enhancing utilities per coding guidelines.
71-72: LGTM: Consistent state tracking with lastUpdate.The
lastUpdatetimestamp is correctly maintained on every event, enabling accurate stale detection. The error field is properly preserved from the WebSocket message.
| task_name: z.string(), | ||
| payload: z.record(z.unknown()), | ||
| status: zTaskStatus, | ||
| result: zDownloadFileResult.optional(), |
There was a problem hiding this comment.
nit: you could make the a generic and inject the zDownloadFileResult in the getTask function to future proof this a bit
There was a problem hiding this comment.
That's what the caveat at the top is for.
I don't like to make single use things generic. Reusable things, those should be generic.
src/stores/assetDownloadStore.ts
Outdated
|
|
||
| const PROCESSED_TASK_CLEANUP_MS = 60000 | ||
| const MAX_COMPLETED_DOWNLOADS = 10 | ||
| const STALE_THRESHOLD_MS = 30_000 |
There was a problem hiding this comment.
we can maybe make this a little shorter? but probably okay. this is a fallback
There was a problem hiding this comment.
30 seconds does feel like an eternity sometimes. 10 seconds seem better?
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/stores/assetDownloadStore.ts:
- Around line 123-125: The empty catch in src/stores/assetDownloadStore.ts
should capture the error and log it instead of silently swallowing it; change
the block to catch (err) and call the module's logger (or console.error if no
logger exists) with a descriptive message including context (e.g.,
"pollDownloadTask failed" or the method name) and the error object so
network/malformed-response issues are visible; optionally distinguish expected
"task not ready/not found" cases from unexpected errors before deciding whether
to swallow or rethrow.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/stores/assetDownloadStore.ts
🧰 Additional context used
📓 Path-based instructions (7)
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.jsonLeverage VueUse functions for performance-enhancing utilities
Files:
src/stores/assetDownloadStore.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: Minimize the surface area (exported values) of each module and composable
Use es-toolkit for utility functions
Files:
src/stores/assetDownloadStore.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; do not mix inlinetypeimports in the same statement
Sort and group imports by plugin; runpnpm formatbefore committing
Derive component types usingvue-component-type-helpers(ComponentProps,ComponentSlots) instead of separate type files
Code should be well-designed with clear names for everything; write code that is expressive and self-documenting
Avoid redundant comments and clean up code as you go; comments should explain why, not what
Ask if there is a simpler way to implement functionality; refactor complex code to simplify it
Minimize nesting depth (e.g.,if () { ... }orfor () { ... }); watch for arrow anti-pattern
Watch out for code smells and refactor to avoid them
Never useanytype; use proper TypeScript types
Never useas anytype assertions; fix the underlying type issue instead
Indent with 2 spaces; use single quotes; no trailing semicolons; max line width 80 (per .prettierrc)
Complex type definitions used in multiple related places should be extracted and named for reuse
Implement proper error handling
Files:
src/stores/assetDownloadStore.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/stores/assetDownloadStore.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/stores/assetDownloadStore.ts
src/stores/**/*Store.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name Pinia store files with the
*Store.tssuffix
Files:
src/stores/assetDownloadStore.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Keep functions short and functional; use function declarations instead of function expressions when possible
Avoid mutable state; prefer immutability and assignment at point of declaration
Favor pure functions, especially testable ones
Files:
src/stores/assetDownloadStore.ts
🧠 Learnings (10)
📚 Learning: 2026-01-08T21:43:58.213Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7897
File: src/stores/assetDownloadStore.ts:117-117
Timestamp: 2026-01-08T21:43:58.213Z
Learning: In src/stores/assetDownloadStore.ts, the api.addEventListener('asset_download', handleAssetDownload) call at module level is intentional and should persist for the application lifetime since it's a singleton Pinia store managing global WebSocket events.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2026-01-06T19:20:56.167Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7746
File: src/platform/assets/services/assetService.ts:484-491
Timestamp: 2026-01-06T19:20:56.167Z
Learning: In `src/platform/assets/services/assetService.ts`, prefer using `schema.safeParse()` over `schema.parse()` for validating API responses to avoid throwing ZodError with internal schema details; follow the existing pattern: call `safeParse()`, check `result.success`, and use `fromZodError(result.error)` to format error messages for logging while throwing user-friendly errors.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2026-01-09T00:50:57.103Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T00:50:57.103Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Implement proper error handling
Applied to files:
src/stores/assetDownloadStore.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} : Implement proper error handling
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2026-01-06T19:17:12.355Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7746
File: src/platform/assets/composables/useUploadModelWizard.ts:207-219
Timestamp: 2026-01-06T19:17:12.355Z
Learning: In `src/platform/assets/composables/useUploadModelWizard.ts`, the `uploadModel()` function should have a concurrency guard at the beginning that checks if `isUploading.value` is already true and returns false early to prevent race conditions from multiple rapid invocations.
Applied to files:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.ts
🧬 Code graph analysis (1)
src/stores/assetDownloadStore.ts (1)
src/platform/tasks/services/taskService.ts (1)
taskService(70-70)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (7)
src/stores/assetDownloadStore.ts (7)
1-7: LGTM!Imports are properly organized with
import typeseparated as required. Good use of VueUse'suseIntervalFnfor the polling mechanism.
18-18: LGTM!The
lastUpdatetimestamp addition enables staleness detection for the polling fallback.
28-29: LGTM!Constants are appropriately defined using numeric separators for readability.
54-91: LGTM!Good improvements to
handleAssetDownload:
- Terminal state guard prevents re-processing of completed/failed downloads
lastUpdatetimestamp enables staleness tracking- Completed downloads array properly capped at
MAX_COMPLETED_DOWNLOADS
131-140: LGTM!Clean polling lifecycle management using VueUse's
useIntervalFnwith reactive start/stop based on active downloads. Theimmediate: falseensures polling only starts when needed. Based on learnings, the singleton store pattern means cleanup isn't required.
150-159: LGTM!Good encapsulation: polling mechanism kept internal while maintaining a clean, minimal public API.
103-121: No action needed—all accessed properties are properly defined in theTaskResponsetype.The
TaskResponsetype includesstatus,result, anderror_messageas verified in the taskService definition. All properties accessed in the code snippet (asset_id,filename,bytes_downloaded,error) are correctly defined in thezDownloadFileResultschema. Optional chaining is appropriately used for the optionalresultobject.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/stores/assetDownloadStore.ts:
- Around line 123-125: In assetDownloadStore.ts replace the empty catch block
that currently swallows errors (the catch following the try that comments "Task
not ready or not found") with a catch (err) { ... } that logs the error at debug
level (e.g., using your store/logger instance or console.debug) including a
short contextual message and the error object, then continue (do not rethrow) so
polling proceeds but real failures are visible for troubleshooting.
- Around line 113-115: The code assigns bytes_total and bytes_downloaded from
the same value; change the assignments so bytes_total uses result?.bytes_total
?? download.bytesTotal and bytes_downloaded uses result?.bytes_downloaded ??
download.bytesDownloaded (update the field names to the correct result/download
properties) so total and downloaded are sourced from their respective fields
instead of both using the downloaded/bytesTotal value.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/schemas/apiSchema.tssrc/stores/assetDownloadStore.ts
🧰 Additional context used
📓 Path-based instructions (7)
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.jsonLeverage VueUse functions for performance-enhancing utilities
Files:
src/schemas/apiSchema.tssrc/stores/assetDownloadStore.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: Minimize the surface area (exported values) of each module and composable
Use es-toolkit for utility functions
Files:
src/schemas/apiSchema.tssrc/stores/assetDownloadStore.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; do not mix inlinetypeimports in the same statement
Sort and group imports by plugin; runpnpm formatbefore committing
Derive component types usingvue-component-type-helpers(ComponentProps,ComponentSlots) instead of separate type files
Code should be well-designed with clear names for everything; write code that is expressive and self-documenting
Avoid redundant comments and clean up code as you go; comments should explain why, not what
Ask if there is a simpler way to implement functionality; refactor complex code to simplify it
Minimize nesting depth (e.g.,if () { ... }orfor () { ... }); watch for arrow anti-pattern
Watch out for code smells and refactor to avoid them
Never useanytype; use proper TypeScript types
Never useas anytype assertions; fix the underlying type issue instead
Indent with 2 spaces; use single quotes; no trailing semicolons; max line width 80 (per .prettierrc)
Complex type definitions used in multiple related places should be extracted and named for reuse
Implement proper error handling
Files:
src/schemas/apiSchema.tssrc/stores/assetDownloadStore.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/schemas/apiSchema.tssrc/stores/assetDownloadStore.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Keep functions short and functional; use function declarations instead of function expressions when possible
Avoid mutable state; prefer immutability and assignment at point of declaration
Favor pure functions, especially testable ones
Files:
src/schemas/apiSchema.tssrc/stores/assetDownloadStore.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/stores/assetDownloadStore.ts
src/stores/**/*Store.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name Pinia store files with the
*Store.tssuffix
Files:
src/stores/assetDownloadStore.ts
🧠 Learnings (20)
📚 Learning: 2026-01-06T19:20:56.167Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7746
File: src/platform/assets/services/assetService.ts:484-491
Timestamp: 2026-01-06T19:20:56.167Z
Learning: In `src/platform/assets/services/assetService.ts`, prefer using `schema.safeParse()` over `schema.parse()` for validating API responses to avoid throwing ZodError with internal schema details; follow the existing pattern: call `safeParse()`, check `result.success`, and use `fromZodError(result.error)` to format error messages for logging while throwing user-friendly errors.
Applied to files:
src/schemas/apiSchema.tssrc/stores/assetDownloadStore.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:
src/schemas/apiSchema.tssrc/stores/assetDownloadStore.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:
src/schemas/apiSchema.tssrc/stores/assetDownloadStore.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:
src/schemas/apiSchema.tssrc/stores/assetDownloadStore.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:
src/schemas/apiSchema.tssrc/stores/assetDownloadStore.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:
src/schemas/apiSchema.tssrc/stores/assetDownloadStore.ts
📚 Learning: 2026-01-08T21:43:58.213Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7897
File: src/stores/assetDownloadStore.ts:117-117
Timestamp: 2026-01-08T21:43:58.213Z
Learning: In src/stores/assetDownloadStore.ts, the api.addEventListener('asset_download', handleAssetDownload) call at module level is intentional and should persist for the application lifetime since it's a singleton Pinia store managing global WebSocket events.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2026-01-09T00:50:57.103Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T00:50:57.103Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Implement proper error handling
Applied to files:
src/stores/assetDownloadStore.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} : Implement proper error handling
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-22T21:36:46.909Z
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:46.909Z
Learning: In the Comfy-Org/ComfyUI_frontend repository test files: Do not stub primitive UI components or customized primitive components (e.g., Button). Instead, import and register the real components in test setup. This ensures tests accurately reflect production behavior and component API usage.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-09T03:50:03.127Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/platform/updates/components/WhatsNewPopup.vue:5-13
Timestamp: 2025-12-09T03:50:03.127Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, when avoiding PrimeVue components, prefer using the repo's common button components from src/components/button/ (IconButton.vue, TextButton.vue, IconTextButton.vue) rather than plain HTML buttons. These components wrap PrimeVue with custom design system styling.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-09T04:35:43.971Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/locales/en/main.json:774-780
Timestamp: 2025-12-09T04:35:43.971Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, locale files other than `src/locales/en/main.json` are generated automatically on every release. Developers only need to add English (en) key/values in `src/locales/en/main.json` when making PRs; manual updates to other locale files (fr, ja, ko, ru, zh, zh-TW, es, ar, tr, etc.) are not required and should not be suggested in reviews.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-16T22:22:06.840Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: pnpm-workspace.yaml:50-50
Timestamp: 2025-12-16T22:22:06.840Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, the project uses cva1.0.0-beta.4 (beta version) by deliberate choice. Do not suggest replacing it with class-variance-authority or flagging it as a beta dependency concern.
Applied to files:
src/stores/assetDownloadStore.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 the Comfy-Org/ComfyUI_frontend repository test files: When testing components, import the real type definitions from the component files instead of duplicating interface definitions in the test files. This prevents type drift and maintains consistency.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-05T06:11:09.383Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7177
File: src/platform/assets/components/UploadModelFooter.vue:72-78
Timestamp: 2025-12-05T06:11:09.383Z
Learning: For the ComfyUI_frontend repository, avoid suggesting comments that would be redundant when the code is already self-explanatory through descriptive naming (e.g., filenames, prop names, aria-labels). The project prefers clean code without unnecessary documentation comments.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-22T21:36:16.031Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: src/platform/cloud/subscription/components/PricingTable.vue:185-201
Timestamp: 2025-12-22T21:36:16.031Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, avoid creating single-use variants for the Button component (and other UI components). Variants should be reusable across multiple use cases. It's acceptable to use a mix of variant props and manual Tailwind classes temporarily when a specific styling need exists in only one place, with consolidation deferred to a later phase when patterns emerge.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-09T21:40:19.792Z
Learnt from: benceruleanlu
Repo: Comfy-Org/ComfyUI_frontend PR: 7297
File: src/components/actionbar/ComfyActionbar.vue:33-43
Timestamp: 2025-12-09T21:40:19.792Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, inline Tailwind CSS class strings, even when long, are acceptable and preferred over extracting them to computed properties when the classes are static. This is a common Tailwind pattern and doesn't need to be flagged as a readability issue.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-10T03:09:19.636Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7303
File: src/components/topbar/CurrentUserPopover.test.ts:199-205
Timestamp: 2025-12-10T03:09:19.636Z
Learning: For test files in the Comfy-Org/ComfyUI_frontend repository: When writing tests, prefer selecting elements by accessible properties (text content, aria-label, role, accessible name) over data-testid attributes. This ensures tests verify actual user-facing behavior and accessibility compliance.
Applied to files:
src/stores/assetDownloadStore.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: Implement proper error handling in stores
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2026-01-06T19:17:12.355Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7746
File: src/platform/assets/composables/useUploadModelWizard.ts:207-219
Timestamp: 2026-01-06T19:17:12.355Z
Learning: In `src/platform/assets/composables/useUploadModelWizard.ts`, the `uploadModel()` function should have a concurrency guard at the beginning that checks if `isUploading.value` is already true and returns false early to prevent race conditions from multiple rapid invocations.
Applied to files:
src/stores/assetDownloadStore.ts
🧬 Code graph analysis (1)
src/stores/assetDownloadStore.ts (1)
src/platform/tasks/services/taskService.ts (1)
taskService(70-70)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: deploy-and-comment
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: test
- GitHub Check: collect
🔇 Additional comments (7)
src/schemas/apiSchema.ts (1)
138-147: LGTM - Schema change aligns with store interface.The
asset_idfield is now optional, which correctly matches theassetId?: stringfield in theAssetDownloadinterface. This supports the polling fallback mechanism where the asset_id may not be present in all WebSocket messages.src/stores/assetDownloadStore.ts (6)
28-29: Verify intended threshold and interval values.The PR description states a 30-second stale threshold and 15-second polling interval, but the code uses 10 seconds for both. Please confirm which values are intended.
54-61: LGTM - Terminal state guard prevents redundant processing.The early return for downloads already in terminal state (
completed/failed) correctly prevents duplicate event processing, which is especially important when the polling fallback synthesizes events for stale downloads.
63-75: LGTM - Download tracking with lastUpdate timestamp.The
lastUpdatefield is correctly set on every WebSocket event, enabling the staleness detection in the polling mechanism.
77-91: LGTM - Completed downloads tracking with bounded history.The immutable array pattern ensures proper Vue reactivity, and the
shift()call correctly maintains a bounded history of the most recent completed downloads.
131-142: LGTM - Polling lifecycle management.Good use of VueUse's
useIntervalFnwithimmediate: falseto defer polling until needed. Thewatchcorrectly starts/stops polling based on active downloads, avoiding unnecessary polling when idle. Based on learnings, the module-leveladdEventListeneris intentional for this singleton store.
150-159: LGTM - Clean public interface.The store correctly exposes only the necessary reactive state and actions, keeping internal implementation details (
handleAssetDownload,pollStaleDownloads) private.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/stores/assetDownloadStore.ts:
- Around line 137-140: The watcher on hasActiveDownloads currently only responds
to changes and will miss handling downloads already active at initialization;
modify the watch call for hasActiveDownloads to pass the options object with
immediate: true so the callback runs once immediately and invokes resume() or
pause() based on the current value, ensuring existing active downloads are
polled on startup (target the watch(hasActiveDownloads, ...) invocation and
adjust its call signature to include { immediate: true }).
- Around line 28-29: The constants STALE_THRESHOLD_MS and POLL_INTERVAL_MS are
set to 10_000 but the PR summary specifies a 30s stale threshold and 15s poll
interval; update STALE_THRESHOLD_MS to 30_000 and POLL_INTERVAL_MS to 15_000
(replace the numeric values where those constants are declared) so the code
matches the documented behavior.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/stores/assetDownloadStore.ts
🧰 Additional context used
📓 Path-based instructions (7)
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.jsonLeverage VueUse functions for performance-enhancing utilities
Files:
src/stores/assetDownloadStore.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: Minimize the surface area (exported values) of each module and composable
Use es-toolkit for utility functions
Files:
src/stores/assetDownloadStore.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; do not mix inlinetypeimports in the same statement
Sort and group imports by plugin; runpnpm formatbefore committing
Derive component types usingvue-component-type-helpers(ComponentProps,ComponentSlots) instead of separate type files
Code should be well-designed with clear names for everything; write code that is expressive and self-documenting
Avoid redundant comments and clean up code as you go; comments should explain why, not what
Ask if there is a simpler way to implement functionality; refactor complex code to simplify it
Minimize nesting depth (e.g.,if () { ... }orfor () { ... }); watch for arrow anti-pattern
Watch out for code smells and refactor to avoid them
Never useanytype; use proper TypeScript types
Never useas anytype assertions; fix the underlying type issue instead
Indent with 2 spaces; use single quotes; no trailing semicolons; max line width 80 (per .prettierrc)
Complex type definitions used in multiple related places should be extracted and named for reuse
Implement proper error handling
Files:
src/stores/assetDownloadStore.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/stores/assetDownloadStore.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/stores/assetDownloadStore.ts
src/stores/**/*Store.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name Pinia store files with the
*Store.tssuffix
Files:
src/stores/assetDownloadStore.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Keep functions short and functional; use function declarations instead of function expressions when possible
Avoid mutable state; prefer immutability and assignment at point of declaration
Favor pure functions, especially testable ones
Files:
src/stores/assetDownloadStore.ts
🧠 Learnings (19)
📚 Learning: 2026-01-08T21:43:58.213Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7897
File: src/stores/assetDownloadStore.ts:117-117
Timestamp: 2026-01-08T21:43:58.213Z
Learning: In src/stores/assetDownloadStore.ts, the api.addEventListener('asset_download', handleAssetDownload) call at module level is intentional and should persist for the application lifetime since it's a singleton Pinia store managing global WebSocket events.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2026-01-06T19:20:56.167Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7746
File: src/platform/assets/services/assetService.ts:484-491
Timestamp: 2026-01-06T19:20:56.167Z
Learning: In `src/platform/assets/services/assetService.ts`, prefer using `schema.safeParse()` over `schema.parse()` for validating API responses to avoid throwing ZodError with internal schema details; follow the existing pattern: call `safeParse()`, check `result.success`, and use `fromZodError(result.error)` to format error messages for logging while throwing user-friendly errors.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2026-01-09T00:50:57.103Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T00:50:57.103Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Implement proper error handling
Applied to files:
src/stores/assetDownloadStore.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} : Implement proper error handling
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-22T21:36:46.909Z
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:46.909Z
Learning: In the Comfy-Org/ComfyUI_frontend repository test files: Do not stub primitive UI components or customized primitive components (e.g., Button). Instead, import and register the real components in test setup. This ensures tests accurately reflect production behavior and component API usage.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-09T03:50:03.127Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/platform/updates/components/WhatsNewPopup.vue:5-13
Timestamp: 2025-12-09T03:50:03.127Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, when avoiding PrimeVue components, prefer using the repo's common button components from src/components/button/ (IconButton.vue, TextButton.vue, IconTextButton.vue) rather than plain HTML buttons. These components wrap PrimeVue with custom design system styling.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-09T04:35:43.971Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/locales/en/main.json:774-780
Timestamp: 2025-12-09T04:35:43.971Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, locale files other than `src/locales/en/main.json` are generated automatically on every release. Developers only need to add English (en) key/values in `src/locales/en/main.json` when making PRs; manual updates to other locale files (fr, ja, ko, ru, zh, zh-TW, es, ar, tr, etc.) are not required and should not be suggested in reviews.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-16T22:22:06.840Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: pnpm-workspace.yaml:50-50
Timestamp: 2025-12-16T22:22:06.840Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, the project uses cva1.0.0-beta.4 (beta version) by deliberate choice. Do not suggest replacing it with class-variance-authority or flagging it as a beta dependency concern.
Applied to files:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.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 the Comfy-Org/ComfyUI_frontend repository test files: When testing components, import the real type definitions from the component files instead of duplicating interface definitions in the test files. This prevents type drift and maintains consistency.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-05T06:11:09.383Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7177
File: src/platform/assets/components/UploadModelFooter.vue:72-78
Timestamp: 2025-12-05T06:11:09.383Z
Learning: For the ComfyUI_frontend repository, avoid suggesting comments that would be redundant when the code is already self-explanatory through descriptive naming (e.g., filenames, prop names, aria-labels). The project prefers clean code without unnecessary documentation comments.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-22T21:36:16.031Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: src/platform/cloud/subscription/components/PricingTable.vue:185-201
Timestamp: 2025-12-22T21:36:16.031Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, avoid creating single-use variants for the Button component (and other UI components). Variants should be reusable across multiple use cases. It's acceptable to use a mix of variant props and manual Tailwind classes temporarily when a specific styling need exists in only one place, with consolidation deferred to a later phase when patterns emerge.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-09T21:40:19.792Z
Learnt from: benceruleanlu
Repo: Comfy-Org/ComfyUI_frontend PR: 7297
File: src/components/actionbar/ComfyActionbar.vue:33-43
Timestamp: 2025-12-09T21:40:19.792Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, inline Tailwind CSS class strings, even when long, are acceptable and preferred over extracting them to computed properties when the classes are static. This is a common Tailwind pattern and doesn't need to be flagged as a readability issue.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-10T03:09:19.636Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7303
File: src/components/topbar/CurrentUserPopover.test.ts:199-205
Timestamp: 2025-12-10T03:09:19.636Z
Learning: For test files in the Comfy-Org/ComfyUI_frontend repository: When writing tests, prefer selecting elements by accessible properties (text content, aria-label, role, accessible name) over data-testid attributes. This ensures tests verify actual user-facing behavior and accessibility compliance.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2026-01-06T19:17:12.355Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7746
File: src/platform/assets/composables/useUploadModelWizard.ts:207-219
Timestamp: 2026-01-06T19:17:12.355Z
Learning: In `src/platform/assets/composables/useUploadModelWizard.ts`, the `uploadModel()` function should have a concurrency guard at the beginning that checks if `isUploading.value` is already true and returns false early to prevent race conditions from multiple rapid invocations.
Applied to files:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.ts
🧬 Code graph analysis (1)
src/stores/assetDownloadStore.ts (1)
src/platform/tasks/services/taskService.ts (1)
taskService(70-70)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test
- GitHub Check: lint-and-format
- GitHub Check: setup
- GitHub Check: collect
🔇 Additional comments (2)
src/stores/assetDownloadStore.ts (2)
56-61: LGTM! Prevents duplicate processing.The terminal state guard correctly prevents reprocessing downloads that have already reached a completed or failed state, which is essential for the polling reconciliation mechanism.
72-72: LGTM! Clean timestamp tracking and sliding window cleanup.The
lastUpdatetimestamp enables stale download detection, and the refactored sliding window approach forcompletedDownloadsis cleaner than the previous bookkeeping mechanism.Also applies to: 80-86
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/stores/assetDownloadStore.ts:
- Around line 80-85: Replace the mutation-with-shift pattern on
completedDownloads.value with a pure slice-based truncation: after building the
new array (currently assigned to updated), set completedDownloads.value to the
last MAX_COMPLETED_DOWNLOADS items by using Array.prototype.slice with a
negative start index so no in-place mutation (shift) happens; keep the same
object shape ({ taskId: data.task_id, modelType, timestamp: Date.now() }) and
the same symbols (completedDownloads.value, MAX_COMPLETED_DOWNLOADS, updated) to
locate and apply the change.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/stores/assetDownloadStore.ts
🧰 Additional context used
📓 Path-based instructions (7)
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.jsonLeverage VueUse functions for performance-enhancing utilities
Files:
src/stores/assetDownloadStore.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: Minimize the surface area (exported values) of each module and composable
Use es-toolkit for utility functions
Files:
src/stores/assetDownloadStore.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; do not mix inlinetypeimports in the same statement
Sort and group imports by plugin; runpnpm formatbefore committing
Derive component types usingvue-component-type-helpers(ComponentProps,ComponentSlots) instead of separate type files
Code should be well-designed with clear names for everything; write code that is expressive and self-documenting
Avoid redundant comments and clean up code as you go; comments should explain why, not what
Ask if there is a simpler way to implement functionality; refactor complex code to simplify it
Minimize nesting depth (e.g.,if () { ... }orfor () { ... }); watch for arrow anti-pattern
Watch out for code smells and refactor to avoid them
Never useanytype; use proper TypeScript types
Never useas anytype assertions; fix the underlying type issue instead
Indent with 2 spaces; use single quotes; no trailing semicolons; max line width 80 (per .prettierrc)
Complex type definitions used in multiple related places should be extracted and named for reuse
Implement proper error handling
Files:
src/stores/assetDownloadStore.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/stores/assetDownloadStore.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/stores/assetDownloadStore.ts
src/stores/**/*Store.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name Pinia store files with the
*Store.tssuffix
Files:
src/stores/assetDownloadStore.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Keep functions short and functional; use function declarations instead of function expressions when possible
Avoid mutable state; prefer immutability and assignment at point of declaration
Favor pure functions, especially testable ones
Files:
src/stores/assetDownloadStore.ts
🧠 Learnings (22)
📓 Common learnings
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7926
File: src/stores/assetDownloadStore.ts:101-126
Timestamp: 2026-01-09T23:59:04.777Z
Learning: In src/stores/assetDownloadStore.ts, the empty catch block in pollSingleDownload within pollStaleDownloads is intentional. Do not suggest adding error logging or handling for polling failures in this function.
📚 Learning: 2026-01-09T23:59:04.777Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7926
File: src/stores/assetDownloadStore.ts:101-126
Timestamp: 2026-01-09T23:59:04.777Z
Learning: In src/stores/assetDownloadStore.ts, the empty catch block in pollSingleDownload within pollStaleDownloads is intentional. Do not suggest adding error logging or error handling for polling failures in this function; treat it as a deliberate swallow of errors to avoid affecting poll logic or user-visible behavior. Reviewers should refrain from proposing fixes to this catch block for this function.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2026-01-08T21:43:58.213Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7897
File: src/stores/assetDownloadStore.ts:117-117
Timestamp: 2026-01-08T21:43:58.213Z
Learning: In src/stores/assetDownloadStore.ts, the api.addEventListener('asset_download', handleAssetDownload) call at module level is intentional and should persist for the application lifetime since it's a singleton Pinia store managing global WebSocket events.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2026-01-06T19:20:56.167Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7746
File: src/platform/assets/services/assetService.ts:484-491
Timestamp: 2026-01-06T19:20:56.167Z
Learning: In `src/platform/assets/services/assetService.ts`, prefer using `schema.safeParse()` over `schema.parse()` for validating API responses to avoid throwing ZodError with internal schema details; follow the existing pattern: call `safeParse()`, check `result.success`, and use `fromZodError(result.error)` to format error messages for logging while throwing user-friendly errors.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2026-01-09T00:50:57.103Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T00:50:57.103Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Implement proper error handling
Applied to files:
src/stores/assetDownloadStore.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} : Implement proper error handling
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-22T21:36:46.909Z
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:46.909Z
Learning: In the Comfy-Org/ComfyUI_frontend repository test files: Do not stub primitive UI components or customized primitive components (e.g., Button). Instead, import and register the real components in test setup. This ensures tests accurately reflect production behavior and component API usage.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-09T03:50:03.127Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/platform/updates/components/WhatsNewPopup.vue:5-13
Timestamp: 2025-12-09T03:50:03.127Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, when avoiding PrimeVue components, prefer using the repo's common button components from src/components/button/ (IconButton.vue, TextButton.vue, IconTextButton.vue) rather than plain HTML buttons. These components wrap PrimeVue with custom design system styling.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-09T04:35:43.971Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/locales/en/main.json:774-780
Timestamp: 2025-12-09T04:35:43.971Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, locale files other than `src/locales/en/main.json` are generated automatically on every release. Developers only need to add English (en) key/values in `src/locales/en/main.json` when making PRs; manual updates to other locale files (fr, ja, ko, ru, zh, zh-TW, es, ar, tr, etc.) are not required and should not be suggested in reviews.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-16T22:22:06.840Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: pnpm-workspace.yaml:50-50
Timestamp: 2025-12-16T22:22:06.840Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, the project uses cva1.0.0-beta.4 (beta version) by deliberate choice. Do not suggest replacing it with class-variance-authority or flagging it as a beta dependency concern.
Applied to files:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.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 the Comfy-Org/ComfyUI_frontend repository test files: When testing components, import the real type definitions from the component files instead of duplicating interface definitions in the test files. This prevents type drift and maintains consistency.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-05T06:11:09.383Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7177
File: src/platform/assets/components/UploadModelFooter.vue:72-78
Timestamp: 2025-12-05T06:11:09.383Z
Learning: For the ComfyUI_frontend repository, avoid suggesting comments that would be redundant when the code is already self-explanatory through descriptive naming (e.g., filenames, prop names, aria-labels). The project prefers clean code without unnecessary documentation comments.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-22T21:36:16.031Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: src/platform/cloud/subscription/components/PricingTable.vue:185-201
Timestamp: 2025-12-22T21:36:16.031Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, avoid creating single-use variants for the Button component (and other UI components). Variants should be reusable across multiple use cases. It's acceptable to use a mix of variant props and manual Tailwind classes temporarily when a specific styling need exists in only one place, with consolidation deferred to a later phase when patterns emerge.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-09T21:40:19.792Z
Learnt from: benceruleanlu
Repo: Comfy-Org/ComfyUI_frontend PR: 7297
File: src/components/actionbar/ComfyActionbar.vue:33-43
Timestamp: 2025-12-09T21:40:19.792Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, inline Tailwind CSS class strings, even when long, are acceptable and preferred over extracting them to computed properties when the classes are static. This is a common Tailwind pattern and doesn't need to be flagged as a readability issue.
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2025-12-10T03:09:19.636Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7303
File: src/components/topbar/CurrentUserPopover.test.ts:199-205
Timestamp: 2025-12-10T03:09:19.636Z
Learning: For test files in the Comfy-Org/ComfyUI_frontend repository: When writing tests, prefer selecting elements by accessible properties (text content, aria-label, role, accessible name) over data-testid attributes. This ensures tests verify actual user-facing behavior and accessibility compliance.
Applied to files:
src/stores/assetDownloadStore.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: Implement proper error handling in stores
Applied to files:
src/stores/assetDownloadStore.ts
📚 Learning: 2026-01-06T19:17:12.355Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7746
File: src/platform/assets/composables/useUploadModelWizard.ts:207-219
Timestamp: 2026-01-06T19:17:12.355Z
Learning: In `src/platform/assets/composables/useUploadModelWizard.ts`, the `uploadModel()` function should have a concurrency guard at the beginning that checks if `isUploading.value` is already true and returns false early to prevent race conditions from multiple rapid invocations.
Applied to files:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.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:
src/stores/assetDownloadStore.ts
🧬 Code graph analysis (1)
src/stores/assetDownloadStore.ts (1)
src/platform/tasks/services/taskService.ts (1)
taskService(70-70)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: collect
- GitHub Check: test
🔇 Additional comments (7)
src/stores/assetDownloadStore.ts (7)
1-7: LGTM!Imports correctly leverage VueUse for the polling utility and follow the guideline of using separate
import typestatements.
9-19: LGTM!Interface changes correctly add
lastUpdatefor staleness tracking and makeassetIdoptional to align with the updated schema.
28-29: LGTM!Constants are well-named and use numeric separators for readability. The 10-second thresholds align with the PR objectives.
58-61: LGTM!The terminal state guard is essential for preventing duplicate processing when both WebSocket messages and polling may report the same completion. Good defensive programming.
93-129: LGTM!The polling logic correctly identifies stale downloads, queries the task service, and synthesizes completion events. The reuse of
handleAssetDownloadensures consistent state transitions, and the terminal state guard in that function makes concurrent polling safe.
131-144: LGTM!The combination of
useIntervalFnwithimmediate: falseandwatchwithimmediate: truecorrectly ensures polling starts immediately when there are already-active downloads on initialization, and properly pauses when no active downloads remain. Good use of VueUse per guidelines.
146-163: LGTM!The event listener registration at module level is appropriate for a singleton Pinia store (per learnings), and the public interface is minimal, keeping the polling implementation internal.
| const updated = [ | ||
| ...completedDownloads.value, | ||
| { taskId: data.task_id, modelType, timestamp: Date.now() } | ||
| ] | ||
| if (updated.length > MAX_COMPLETED_DOWNLOADS) updated.shift() | ||
| completedDownloads.value = updated |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider a more functional approach for array truncation.
The current pattern creates a new array then mutates it with shift(). Per guidelines preferring immutability, a slice-based approach is cleaner.
♻️ Suggested refactor
- const updated = [
- ...completedDownloads.value,
- { taskId: data.task_id, modelType, timestamp: Date.now() }
- ]
- if (updated.length > MAX_COMPLETED_DOWNLOADS) updated.shift()
- completedDownloads.value = updated
+ const newEntry = { taskId: data.task_id, modelType, timestamp: Date.now() }
+ const all = [...completedDownloads.value, newEntry]
+ completedDownloads.value = all.slice(-MAX_COMPLETED_DOWNLOADS)🤖 Prompt for AI Agents
In @src/stores/assetDownloadStore.ts around lines 80 - 85, Replace the
mutation-with-shift pattern on completedDownloads.value with a pure slice-based
truncation: after building the new array (currently assigned to updated), set
completedDownloads.value to the last MAX_COMPLETED_DOWNLOADS items by using
Array.prototype.slice with a negative start index so no in-place mutation
(shift) happens; keep the same object shape ({ taskId: data.task_id, modelType,
timestamp: Date.now() }) and the same symbols (completedDownloads.value,
MAX_COMPLETED_DOWNLOADS, updated) to locate and apply the change.
luke-mino-altherr
added
needs-backport
|
Adds a polling fallback mechanism to recover from dropped WebSocket
messages during model downloads.
When downloading models via the asset download service, status updates
are received over WebSocket. Sometimes these messages are dropped
(network issues, reconnection, etc.), causing downloads to appear
"stuck" even when they've completed on the backend.
Periodically poll for stale downloads using the existing REST API:
- Track `lastUpdate` timestamp on each download
- Downloads without updates for 10s are considered "stale"
- Poll stale downloads every 10s via `GET /tasks/{task_id}` to check if
the asset exists
- If the asset exists with size > 0, mark the download as completed
- Added `lastUpdate` field to `AssetDownload` interface
- Use VueUse's `useIntervalFn` with a `watch` to auto start/stop polling
based on active downloads
- Reuse existing `handleAssetDownload` for completion (synthetic event)
- Added 9 unit tests covering the polling behavior
- All existing tests pass
- New tests cover:
- Basic download tracking
- Completion/failure handling
- Duplicate message prevention
- Stale download polling
- Polling error handling
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7926-feat-add-polling-fallback-for-stale-asset-downloads-2e36d73d3650810ea966f5480f08b60c)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Adds a polling fallback mechanism to recover from dropped WebSocket
messages during model downloads.
When downloading models via the asset download service, status updates
are received over WebSocket. Sometimes these messages are dropped
(network issues, reconnection, etc.), causing downloads to appear
"stuck" even when they've completed on the backend.
Periodically poll for stale downloads using the existing REST API:
- Track `lastUpdate` timestamp on each download
- Downloads without updates for 10s are considered "stale"
- Poll stale downloads every 10s via `GET /tasks/{task_id}` to check if
the asset exists
- If the asset exists with size > 0, mark the download as completed
- Added `lastUpdate` field to `AssetDownload` interface
- Use VueUse's `useIntervalFn` with a `watch` to auto start/stop polling
based on active downloads
- Reuse existing `handleAssetDownload` for completion (synthetic event)
- Added 9 unit tests covering the polling behavior
- All existing tests pass
- New tests cover:
- Basic download tracking
- Completion/failure handling
- Duplicate message prevention
- Stale download polling
- Polling error handling
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7926-feat-add-polling-fallback-for-stale-asset-downloads-2e36d73d3650810ea966f5480f08b60c)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…loads (#7981) Backport of #7926 to cloud/1.36 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7981-backport-cloud-1-36-feat-add-polling-fallback-for-stale-asset-downloads-2e76d73d365081a983a4e5a8683ae2c9) by [Unito](https://www.unito.io) Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
5 participants
Summary
Adds a polling fallback mechanism to recover from dropped WebSocket messages during model downloads.
Problem
When downloading models via the asset download service, status updates are received over WebSocket. Sometimes these messages are dropped (network issues, reconnection, etc.), causing downloads to appear "stuck" even when they've completed on the backend.
Solution
Periodically poll for stale downloads using the existing REST API:
lastUpdatetimestamp on each downloadGET /tasks/{task_id}to check if the asset existsImplementation
lastUpdatefield toAssetDownloadinterfaceuseIntervalFnwith awatchto auto start/stop polling based on active downloadshandleAssetDownloadfor completion (synthetic event)Testing
┆Issue is synchronized with this Notion page by Unito