refactor: change asset cache from nodeType-keyed to category-keyed#8433
Merged
refactor: change asset cache from nodeType-keyed to category-keyed#8433
Conversation
- Rename modelStateByKey to modelStateByCategory - Add resolveCategory() to translate nodeType -> 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 This architectural change enables simple cache invalidation by category (e.g., 'checkpoints', 'loras') which will be used for deletion invalidation. Amp-Thread-ID: https://ampcode.com/threads/T-019c08a2-6a9f-770f-994c-ad79d515f6a1 Co-authored-by: Amp <amp@ampcode.com>
dosubot
bot
added
the
size:L
Contributor
📝 WalkthroughWalkthroughRefactors asset caching from per-key (node type/tag) to category-based caches in Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AssetsStore as assetsStore
participant ModelToNode as modelToNodeStore
participant AssetSvc as AssetService
Client->>AssetsStore: getAssets(key)
AssetsStore->>ModelToNode: getCategoryForNodeType(key) / resolveCategory(key)
alt category resolved
AssetsStore->>AssetsStore: check modelStateByCategory[category]
alt cache hit
AssetsStore-->>Client: return cached assets
else cache miss
AssetsStore->>AssetSvc: fetch assets (category, pagination)
AssetSvc-->>AssetsStore: assets[]
AssetsStore->>AssetsStore: update modelStateByCategory[category]
AssetsStore-->>Client: return assets
end
else unknown key
AssetsStore-->>Client: return []
end
Possibly related PRs
Suggested reviewers
✨ 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 Tests:
|
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/29/2026, 10:53:31 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
coderabbitai
bot
requested review from
AustinMroz,
KarryCharon,
Yorha4D,
luke-mino-altherr and
shinshin86
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 26 kB (baseline 26 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 974 kB (baseline 974 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 80.7 kB (baseline 80.7 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 471 kB (baseline 471 kB) • 🟢 -8 BConfiguration panels, inspectors, and settings screens
Status: 12 added / 12 removed User & Accounts — 3.94 kB (baseline 3.94 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 3 added / 3 removed Editors & Dialogs — 2.89 kB (baseline 2.89 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 33.7 kB (baseline 33.7 kB) • ⚪ 0 BReusable component library chunks
Status: 4 added / 4 removed Data & Services — 2.71 MB (baseline 2.7 MB) • 🔴 +1.53 kBStores, services, APIs, and repositories
Status: 8 added / 8 removed Utilities & Hooks — 25.3 kB (baseline 25.3 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 7 added / 7 removed Vendor & Third-Party — 10.7 MB (baseline 10.7 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.1 MB (baseline 7.1 MB) • 🟢 -198 BBundles that do not match a named category
Status: 34 added / 34 removed |
🔧 Auto-fixes AppliedThis PR has been automatically updated to fix linting and formatting issues.
Changes made:
|
![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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/stores/assetsStore.test.ts (1)
584-592: Test uses unmapped node type, potentially not testing intended cache behavior.
'TestLoader'is not in the mock'snodeToCategorymapping, sogetCategoryForNodeTypereturnsundefined, causinggetAssetsto return theEMPTY_ASSETSconstant. The test passes because the same constant reference is returned both times, but this doesn't validate the array cache behavior for populated data.Consider using a mapped node type like
'CheckpointLoaderSimple'after populating the cache:Proposed fix
it('should return cached array on subsequent getAssets calls', () => { const store = useAssetsStore() - const nodeType = 'TestLoader' + const nodeType = 'CheckpointLoaderSimple' + const assets = [createMockAsset('cache-test-1')] + + vi.mocked(assetService.getAssetsForNodeType).mockResolvedValue(assets) + await store.updateModelsForNodeType(nodeType) const firstCall = store.getAssets(nodeType) const secondCall = store.getAssets(nodeType) expect(secondCall).toBe(firstCall) + expect(firstCall).toHaveLength(1) })
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/stores/assetsStore.ts`:
- Around line 383-397: The updateModelsForCategory function can be invoked
concurrently for the same category causing redundant parallel work; add an
explicit guard using the pendingRequestByCategory map to prevent concurrent
execution: when updateModelsForCategory starts, check
pendingRequestByCategory.get(category) and if a pending state/promise exists
either return the existing pending promise or set a dedicated lock token (e.g.,
store the state/promise) so subsequent calls short-circuit; ensure you
update/remove that entry consistently when the work completes or is superseded
(references: updateModelsForCategory, pendingRequestByCategory,
modelStateByCategory, and the existing isStale logic) so behavior remains
correct and maintainable.
- Around line 509-516: When cacheKey is provided but resolveCategory(cacheKey)
returns undefined, the code currently builds categoriesToCheck = [undefined] and
then skips it in the loop; change this to return early instead: after computing
const category = cacheKey ? resolveCategory(cacheKey) : undefined, if cacheKey
&& !category then return (or exit the surrounding function) to avoid creating
categoriesToCheck and running the loop; update any callers/tests if needed. This
touches the symbols: cacheKey, resolveCategory, categoriesToCheck, and
modelStateByCategory.
- Add early return in updateAssetInCache when cacheKey resolves to undefined - Fix test to use mapped node type to properly validate array cache behavior Amp-Thread-ID: https://ampcode.com/threads/T-019c0bac-2144-73cb-a8d5-45146d0c2db9 Co-authored-by: Amp <amp@ampcode.com>
Short-circuit concurrent calls for the same category to prevent redundant parallel work. The pendingRequestByCategory map now tracks all in-progress requests (not just those with existing data), allowing subsequent calls to return immediately instead of starting duplicate fetches. Amp-Thread-ID: https://ampcode.com/threads/T-019c0bac-2144-73cb-a8d5-45146d0c2db9 Co-authored-by: Amp <amp@ampcode.com>
DrJKL
approved these changes
Jan 30, 2026
christian-byrne
added a commit
that referenced
this pull request
Jan 30, 2026
…8433) Refactors the model assets cache in `assetsStore.ts` to be keyed by category (e.g., 'checkpoints', 'loras') instead of nodeType (e.g., 'CheckpointLoaderSimple'). - 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 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 - All existing tests pass with updates - Added new tests for category-keyed cache sharing, invalidateCategory, and unknown node type handling 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 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>
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>
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>
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
3 participants
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
Refactors the model assets cache in
assetsStore.tsto be keyed by category (e.g., 'checkpoints', 'loras') instead of nodeType (e.g., 'CheckpointLoaderSimple').Changes
modelStateByKeytomodelStateByCategoryresolveCategory()helper to translate nodeType to category for cache lookupinvalidateCategory()method for cache invalidationBenefits
Testing
Part of Stack
This is PR 1 of 2 in a stacked PR series:
┆Issue is synchronized with this Notion page by Unito