Fix FIFO behavior in assetsStore #7461
Conversation
dosubot
bot
added
the
size:XS
📝 WalkthroughWalkthroughThe pull request modifies the in-memory history list trimming logic in assetsStore to remove excess items from the start instead of the end, preserving the most recent MAX_HISTORY_ITEMS entries. A minor style update to loadedIds cleanup iteration is also included. Changes
Possibly related PRs
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎭 Playwright Test Results
⏰ Started at: 12/13/2025, 11:22:07 PM UTC 🚀 Running Tests
⏱️ Please wait while tests are running... |
🎨 Storybook Build Status
⏰ Started at: 12/13/2025, 11:22:06 PM UTC 🚀 Building Storybook
⏱️ Please wait while the Storybook build is in progress... |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/stores/assetsStore.ts(1 hunks)
🧰 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.json
Files:
src/stores/assetsStore.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safetyMinimize the surface area (exported values) of each module and composable
Files:
src/stores/assetsStore.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
Files:
src/stores/assetsStore.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/assetsStore.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/stores/assetsStore.ts
src/stores/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Pinia store files must follow the naming pattern
*Store.ts
Files:
src/stores/assetsStore.ts
**/*.{ts,tsx,js,jsx,vue,json}
📄 CodeRabbit inference engine (AGENTS.md)
Code style: Use 2-space indentation, single quotes, no trailing semicolons, and 80-character line width (see
.prettierrc)
Files:
src/stores/assetsStore.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Imports must be sorted and grouped by plugin; runpnpm formatbefore committing
Use TypeScript for type safety; never useanytype - use proper TypeScript types
Never useas anytype assertions; fix the underlying type issue instead
Use es-toolkit for utility functions
Write code that is expressive and self-documenting; avoid comments unless absolutely necessary; do not add or retain redundant comments
Keep functions short and functional
Minimize nesting in code (e.g., deeply nestediforforstatements); apply the Arrow Anti-Pattern principle
Avoid mutable state; prefer immutability and assignment at point of declaration
Favor pure functions, especially testable ones
Files:
src/stores/assetsStore.ts
🧠 Learnings (4)
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/stores/**/*.{ts,tsx} : Maintain clear public interfaces and restrict extension access in stores
Applied to files:
src/stores/assetsStore.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/assetsStore.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/assetsStore.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/assetsStore.ts
| if (allHistoryItems.value.length > MAX_HISTORY_ITEMS) { | ||
| const removed = allHistoryItems.value.slice(MAX_HISTORY_ITEMS) | ||
| allHistoryItems.value = allHistoryItems.value.slice(0, MAX_HISTORY_ITEMS) | ||
| const excess = allHistoryItems.value.length - MAX_HISTORY_ITEMS | ||
| const removed = allHistoryItems.value.slice(0, excess) | ||
| allHistoryItems.value = allHistoryItems.value.slice(excess) | ||
|
|
||
| // Clean up Set | ||
| removed.forEach((item) => loadedIds.delete(item.id)) | ||
| removed.forEach(item => loadedIds.delete(item.id)) | ||
| } |
There was a problem hiding this comment.
Trimming removes the newest items (direction is inverted for a newest-first list).
allHistoryItems is maintained newest-first (Line 86-Line 89, Line 151-Line 163), but the new trimming removes from the start (Line 176-Line 178), which drops the newest entries and keeps older ones—opposite of “remove oldest first / keep most recent MAX_HISTORY_ITEMS”.
Proposed fix (remove oldest from the end, keep newest at the front, and clean loadedIds for actually-removed items):
if (allHistoryItems.value.length > MAX_HISTORY_ITEMS) {
- const excess = allHistoryItems.value.length - MAX_HISTORY_ITEMS
- const removed = allHistoryItems.value.slice(0, excess)
- allHistoryItems.value = allHistoryItems.value.slice(excess)
-
- // Clean up Set
- removed.forEach(item => loadedIds.delete(item.id))
+ const removed = allHistoryItems.value.slice(MAX_HISTORY_ITEMS)
+ allHistoryItems.value = allHistoryItems.value.slice(0, MAX_HISTORY_ITEMS)
+
+ // Clean up Set
+ removed.forEach((item) => loadedIds.delete(item.id))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (allHistoryItems.value.length > MAX_HISTORY_ITEMS) { | |
| const removed = allHistoryItems.value.slice(MAX_HISTORY_ITEMS) | |
| allHistoryItems.value = allHistoryItems.value.slice(0, MAX_HISTORY_ITEMS) | |
| const excess = allHistoryItems.value.length - MAX_HISTORY_ITEMS | |
| const removed = allHistoryItems.value.slice(0, excess) | |
| allHistoryItems.value = allHistoryItems.value.slice(excess) | |
| // Clean up Set | |
| removed.forEach((item) => loadedIds.delete(item.id)) | |
| removed.forEach(item => loadedIds.delete(item.id)) | |
| } | |
| if (allHistoryItems.value.length > MAX_HISTORY_ITEMS) { | |
| const removed = allHistoryItems.value.slice(MAX_HISTORY_ITEMS) | |
| allHistoryItems.value = allHistoryItems.value.slice(0, MAX_HISTORY_ITEMS) | |
| // Clean up Set | |
| removed.forEach((item) => loadedIds.delete(item.id)) | |
| } |
🤖 Prompt for AI Agents
In src/stores/assetsStore.ts around lines 175 to 182, the trimming logic
currently removes from the start (dropping newest items) even though
allHistoryItems is newest-first; change it to remove the oldest items at the end
by keeping the first MAX_HISTORY_ITEMS entries (e.g., set allHistoryItems.value
= allHistoryItems.value.slice(0, MAX_HISTORY_ITEMS)), compute removed =
previous.slice(MAX_HISTORY_ITEMS) to identify actually-removed items, and then
iterate removed to delete their ids from loadedIds so the set stays in sync.
2 participants
Summary
Fix FIFO behavior in
assetsStoreso the oldest history items are removed first when the maximum is exceeded.Changes
allHistoryItemstrimming logic to remove oldest items and maintain correct FIFO order.Review Focus
allHistoryItemsnow correctly removes the oldest entries first.loadedIdsset is cleaned up properly when items are removed.#7458
Screenshots (if applicable)
None
┆Issue is synchronized with this Notion page by Unito