fix: invalidate loader node dropdown cache after model asset deletion#8434
Open
christian-byrne wants to merge 3 commits intomainfrom
Open
fix: invalidate loader node dropdown cache after model asset deletion#8434christian-byrne wants to merge 3 commits intomainfrom
christian-byrne wants to merge 3 commits intomainfrom
Conversation
dosubot
bot
added
the
size:L
Contributor
📝 WalkthroughWalkthroughAfter asset deletions, the action collects model categories from asset tags (excluding 'models', 'input', 'output') and calls Changes
Sequence DiagramsequenceDiagram
actor User
participant MediaAssetActions as "useMediaAssetActions"
participant AssetService as "Asset Service (API)"
participant ModelToNodeStore as "modelToNodeStore"
participant AssetsStore as "assetsStore"
User->>MediaAssetActions: request deleteAssets(assetIds)
MediaAssetActions->>AssetService: call delete APIs
AssetService-->>MediaAssetActions: deletion confirmed
MediaAssetActions->>MediaAssetActions: update history & inputs
MediaAssetActions->>MediaAssetActions: extract tags -> modelCategories (exclude: models,input,output)
loop per category
MediaAssetActions->>ModelToNodeStore: check providers/tags for category
ModelToNodeStore-->>MediaAssetActions: provider/tag info
MediaAssetActions->>AssetsStore: invalidateModelsForCategory(category)
AssetsStore-->>MediaAssetActions: invalidation complete
end
MediaAssetActions-->>User: deletion completed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
🎭 Playwright Tests:
|
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 02/04/2026, 01:45:45 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🔧 Auto-fixes AppliedThis PR has been automatically updated to fix linting and formatting issues.
Changes made:
|
christian-byrne
force-pushed
the
fix/asset-deletion-invalidation
branch
2 times, most recently
from
aa09287 to
3e694a9
Compare
DrJKL
pushed a commit
that referenced
this pull request
Jan 30, 2026
…8433) ## Summary Refactors the model assets cache in `assetsStore.ts` to be keyed by category (e.g., 'checkpoints', 'loras') instead of nodeType (e.g., 'CheckpointLoaderSimple'). ## Changes - Rename `modelStateByKey` to `modelStateByCategory` - Add `resolveCategory()` helper to translate nodeType to category for cache lookup - Multiple node types sharing the same category now share one cache entry - Add `invalidateCategory()` method for cache invalidation - Maintain backwards-compatible public API accepting nodeType - Update tests for new category-keyed behavior ## Benefits 1. **Deduplication**: Same category = same cache entry = single API call 2. **Simple invalidation**: Delete asset with tag 'checkpoints' then invalidate cache 3. **Cleaner mental model**: Store to View reactive flow works naturally ## Testing - All existing tests pass with updates - Added new tests for category-keyed cache sharing, invalidateCategory, and unknown node type handling ## Part of Stack This is **PR 1 of 2** in a stacked PR series: 1. **This PR**: Refactor asset cache to category-keyed (architectural improvement) 2. **[PR 2 #8434](#8434: Fix deletion invalidation ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8433-refactor-change-asset-cache-from-nodeType-keyed-to-category-keyed-2f76d73d365081999b7fda12c9706ab5) by [Unito](https://www.unito.io) --------- Co-authored-by: Subagent 5 <subagent@example.com> Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: GitHub Action <action@github.com>
christian-byrne
force-pushed
the
fix/asset-deletion-invalidation
branch
from
3e694a9 to
630d964
Compare
coderabbitai
bot
requested review from
DrJKL,
KarryCharon,
Yorha4D,
luke-mino-altherr and
shinshin86
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 22.5 kB (baseline 22.5 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 838 kB (baseline 838 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 69 kB (baseline 69 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 410 kB (baseline 410 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 12 added / 12 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 3.47 kB (baseline 3.47 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 37.8 kB (baseline 37.8 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 2.1 MB (baseline 2.1 MB) • 🔴 +1.12 kBStores, services, APIs, and repositories
Status: 11 added / 11 removed Utilities & Hooks — 234 kB (baseline 234 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 12 added / 12 removed Vendor & Third-Party — 9.37 MB (baseline 9.37 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.08 MB (baseline 7.08 MB) • ⚪ 0 BBundles that do not match a named category
Status: 49 added / 49 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/platform/assets/composables/useMediaAssetActions.test.ts`:
- Around line 281-299: The test for useMediaAssetActions.deleteAssets should
also assert that only recognized provider tags trigger invalidation; update the
test (the it block using createMockAsset, mockShowDialog and
actions.deleteAssets) to add an assertion that mockInvalidateCategory was called
exactly once (or alternatively assert it was not called with 'models') so
unrecognized tags like 'models' do not cause extra invalidations.
christian-byrne
commented
Jan 30, 2026
Contributor
Author
christian-byrne
left a comment
christian-byrne
left a comment
There was a problem hiding this comment.
This is not correct if the useMediaAssetActions are not being used for model cards.
Contributor
Author
|
UseMediaActions is actually used by model cards. Regardless, we shouldn't use modelToNode to invalidate, can just create a more logical system to begin laying "models as assets" groundwork |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
christian-byrne
force-pushed
the
fix/asset-deletion-invalidation
branch
from
630d964 to
a84b06a
Compare
christian-byrne
requested review from
a team,
jtydhr88,
ltdrdata and
viva-jinyi
as code owners
snomiao
pushed a commit
that referenced
this pull request
Jan 30, 2026
…8433) ## Summary Refactors the model assets cache in `assetsStore.ts` to be keyed by category (e.g., 'checkpoints', 'loras') instead of nodeType (e.g., 'CheckpointLoaderSimple'). ## Changes - Rename `modelStateByKey` to `modelStateByCategory` - Add `resolveCategory()` helper to translate nodeType to category for cache lookup - Multiple node types sharing the same category now share one cache entry - Add `invalidateCategory()` method for cache invalidation - Maintain backwards-compatible public API accepting nodeType - Update tests for new category-keyed behavior ## Benefits 1. **Deduplication**: Same category = same cache entry = single API call 2. **Simple invalidation**: Delete asset with tag 'checkpoints' then invalidate cache 3. **Cleaner mental model**: Store to View reactive flow works naturally ## Testing - All existing tests pass with updates - Added new tests for category-keyed cache sharing, invalidateCategory, and unknown node type handling ## Part of Stack This is **PR 1 of 2** in a stacked PR series: 1. **This PR**: Refactor asset cache to category-keyed (architectural improvement) 2. **[PR 2 #8434](#8434: Fix deletion invalidation ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8433-refactor-change-asset-cache-from-nodeType-keyed-to-category-keyed-2f76d73d365081999b7fda12c9706ab5) by [Unito](https://www.unito.io) --------- Co-authored-by: Subagent 5 <subagent@example.com> Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: GitHub Action <action@github.com>
christian-byrne
force-pushed
the
fix/asset-deletion-invalidation
branch
from
a84b06a to
b4cd33b
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/platform/assets/composables/useMediaAssetActions.test.ts`:
- Around line 60-71: Remove or clarify the unused getAllNodeProviders mock in
the useModelToNodeStore mock: the deletion invalidation in
useMediaAssetActions.ts relies on assetsStore.hasCategory() (not
modelToNodeStore.getAllNodeProviders()), so either delete the
getAllNodeProviders hoisted mock (and the unused mockGetCategoryForNodeType if
not used elsewhere) or add a short comment next to
useModelToNodeStore/mockGetCategoryForNodeType explaining that
getAllNodeProviders is present only for other tests in this file to avoid
confusion.
In `@src/stores/assetsStore.ts`:
- Around line 647-662: The function removeAssetFromCache and its export are
unused; remove the entire removeAssetFromCache implementation and its export
from the store (the code that references modelStateByCategory, assetsArrayCache
and the tag deletes). Ensure there are no remaining imports/usages (e.g., in
useMediaAssetActions.ts — that flow uses invalidateModelsForCategory instead),
and run a quick grep to confirm no callers reference removeAssetFromCache before
committing.
DrJKL
pushed a commit
that referenced
this pull request
Jan 31, 2026
…8433) ## Summary Refactors the model assets cache in `assetsStore.ts` to be keyed by category (e.g., 'checkpoints', 'loras') instead of nodeType (e.g., 'CheckpointLoaderSimple'). ## Changes - Rename `modelStateByKey` to `modelStateByCategory` - Add `resolveCategory()` helper to translate nodeType to category for cache lookup - Multiple node types sharing the same category now share one cache entry - Add `invalidateCategory()` method for cache invalidation - Maintain backwards-compatible public API accepting nodeType - Update tests for new category-keyed behavior ## Benefits 1. **Deduplication**: Same category = same cache entry = single API call 2. **Simple invalidation**: Delete asset with tag 'checkpoints' then invalidate cache 3. **Cleaner mental model**: Store to View reactive flow works naturally ## Testing - All existing tests pass with updates - Added new tests for category-keyed cache sharing, invalidateCategory, and unknown node type handling ## Part of Stack This is **PR 1 of 2** in a stacked PR series: 1. **This PR**: Refactor asset cache to category-keyed (architectural improvement) 2. **[PR 2 #8434](#8434: Fix deletion invalidation ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8433-refactor-change-asset-cache-from-nodeType-keyed-to-category-keyed-2f76d73d365081999b7fda12c9706ab5) by [Unito](https://www.unito.io) --------- Co-authored-by: Subagent 5 <subagent@example.com> Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: GitHub Action <action@github.com>
- Add hasCategory() to assetsStore to check if a category exists in cache - Use hasCategory() in deleteAssets to determine which model caches to invalidate - Remove modelToNodeStore dependency from useMediaAssetActions for deletion - Fix removeAssetFromCache to use modelStateByCategory instead of modelStateByKey - Add comprehensive tests for cache invalidation on asset deletion Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019c25ee-c9d6-7700-9beb-03f93317d758
christian-byrne
force-pushed
the
fix/asset-deletion-invalidation
branch
from
b4cd33b to
6b86198
Compare
…cate, extract EXCLUDED_TAGS constant, add invalidateModelsForCategory tests Amp-Thread-ID: https://ampcode.com/threads/T-019c25f9-7238-774f-bb65-ea7f4306c4e6
…ory directly Since PR 1 made cache category-keyed, we don't need to iterate through node providers. Just clear the category cache directly - next access triggers refetch automatically. Amp-Thread-ID: https://ampcode.com/threads/T-019c25f9-7238-774f-bb65-ea7f4306c4e6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When deleting a model asset (checkpoint, lora, etc.), the loader node dropdowns now update correctly by invalidating the category-keyed cache.
Problem
After deleting a model asset in the asset browser, the loader node dropdowns (e.g., CheckpointLoaderSimple, LoraLoader) still showed the deleted model. Users had to refresh or re-open the dropdown to see the updated list.
Solution
After successful asset deletion, check each deleted asset's tags for model categories (checkpoints, loras, etc.) and call
assetsStore.invalidateCategory()for each affected category. This triggers a refetch when the dropdown is next accessed.Changes
In
useMediaAssetActions.ts:modelToNodeStore.getAllNodeProviders()invalidateCategory()for each affected categoryIn
useMediaAssetActions.test.ts:useAssetsStoreanduseModelToNodeStoreTesting
Part of Stack
This is PR 2 of 2 in a stacked PR series:
┆Issue is synchronized with this Notion page by Unito
Summary by CodeRabbit
Bug Fixes
Chores
Tests