Skip to content

Fix FIFO behavior in assetsStore #7461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wes-kay wants to merge 1 commit into from
Closed

Fix FIFO behavior in assetsStore #7461

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/stores/assetsStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,11 +173,12 @@ export const useAssetsStore = defineStore('assets', () => {
hasMoreHistory.value = history.History.length === BATCH_SIZE

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))
}
Comment on lines 175 to 182
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

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.

Suggested change
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.


return allHistoryItems.value
Expand Down