Add expandable output stacks to assets list view #8283
Merged
Conversation
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
…on-assets # Conflicts: # src/components/TopMenuSection.test.ts
Contributor
📝 WalkthroughWalkthroughAdds hierarchical output stacks with lazy-loaded children, updates sidebar list/grid to consume stacked assets and expose stack controls, moves anchor-based selection and reconciliation into selection composables/store (persisting lastSelectedAssetId), introduces utilities/tests for resolving previewable outputs, and wires UI to persist last-selected asset on select. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SidebarUI as Sidebar List View
participant OutputStacks as UseOutputStacks
participant JobCache as Job Output Cache
participant Store as AssetSelectionStore
User->>SidebarUI: Click stack indicator (toggle)
SidebarUI->>OutputStacks: toggleStack(asset)
alt first expansion (children not loaded)
OutputStacks->>JobCache: resolveOutputAssetItems(metadata)
JobCache-->>OutputStacks: AssetItem[] (children)
OutputStacks->>OutputStacks: store children, mark promptId expanded
end
OutputStacks->>SidebarUI: update assetItems & selectableAssets
SidebarUI-->>User: render expanded items
User->>SidebarUI: select asset
SidebarUI->>Store: setLastSelectedAssetId(assetId)
Note over OutputStacks,Store: visibleAssets change
OutputStacks->>Store: reconcileSelection(visibleAssets)
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 |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/30/2026, 06:06:52 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Tests: ✅ PassedResults: 507 passed, 0 failed, 0 flaky, 8 skipped (Total: 515) 📊 Browser Reports
|
benceruleanlu
changed the title
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) • 🔴 +57 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.72 MB (baseline 2.71 MB) • 🔴 +9.19 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:
|
benceruleanlu
commented
Jan 24, 2026
Comment on lines
205
to
+209
|
|
||
| // Try asset.id first (OSS), then fall back to metadata (Cloud) | ||
| const metadata = getOutputAssetMetadata(targetAsset.user_metadata) | ||
| const promptId = targetAsset.id || metadata?.promptId | ||
| const promptId = | ||
| metadata?.promptId || | ||
| (getAssetType(targetAsset) === 'output' ? targetAsset.id : undefined) |
Member
Author
There was a problem hiding this comment.
metadata.promptId is set by the frontend mappers here, so the comment was outdated. In cloud, asset.id is the asset UUID (not the job/prompt ID), so preferring metadata.promptId is the correct behavior.
![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: 2
🤖 Fix all issues with AI agents
In `@src/components/sidebar/tabs/AssetsSidebarListView.vue`:
- Around line 101-102: The template calls the async method toggleStack without
handling its promise; update the prop/type and callsite to avoid floating
promises by changing the prop type for toggleStack from () => void to () =>
Promise<void> (or PropType<(asset: Asset) => Promise<void>>) and then explicitly
discard the returned promise in the template call sites (e.g. replace
`@stack-toggle`="toggleStack(item.asset)" with `@stack-toggle`="void
toggleStack(item.asset)"); apply the same change to the other occurrence around
lines 158-159 so all async invocations are typed as Promise<void> and their
promises are explicitly handled/discarded.
In `@src/components/sidebar/tabs/AssetsSidebarTab.vue`:
- Around line 489-492: The helper currently defined as a const arrow function
`handleAssetSelect` should be converted to a named function declaration to match
project conventions and the nearby `handleAssetContextMenu`; rewrite it as
`function handleAssetSelect(asset: AssetItem, assets?: AssetItem[]) { ... }`
preserving the logic that sets `const assetList = assets ??
visibleAssets.value`, finds `index` via `assetList.findIndex(a => a.id ===
asset.id)`, and calls `handleAssetClick(asset, index, assetList)` so references
to `handleAssetClick` and `visibleAssets` remain unchanged.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/platform/assets/composables/useOutputStacks.ts`:
- Around line 65-103: The toggleStack function can leave loadingStackPromptIds
stuck if resolveStackChildren throws; wrap the await resolveStackChildren(asset)
in a try/finally so the block that removes promptId from loadingStackPromptIds
always runs (move the creation of nextLoading before the try, set
loadingStackPromptIds = nextLoading, await resolveStackChildren inside try, and
in finally remove promptId from loadingStackPromptIds), then proceed to set
stackChildrenByPromptId and expandedStackPromptIds only after a successful
children result; reference toggleStack, resolveStackChildren,
loadingStackPromptIds, stackChildrenByPromptId, expandedStackPromptIds, and
promptId when making the change.
Comment on lines
+65
to
+103
| async function toggleStack(asset: AssetItem) { | ||
| const promptId = getStackPromptId(asset) | ||
| if (!promptId) return | ||
|
|
||
| if (expandedStackPromptIds.value.has(promptId)) { | ||
| const next = new Set(expandedStackPromptIds.value) | ||
| next.delete(promptId) | ||
| expandedStackPromptIds.value = next | ||
| return | ||
| } | ||
|
|
||
| if (!stackChildrenByPromptId.value[promptId]?.length) { | ||
| if (loadingStackPromptIds.value.has(promptId)) { | ||
| return | ||
| } | ||
| const nextLoading = new Set(loadingStackPromptIds.value) | ||
| nextLoading.add(promptId) | ||
| loadingStackPromptIds.value = nextLoading | ||
|
|
||
| const children = await resolveStackChildren(asset) | ||
|
|
||
| const afterLoading = new Set(loadingStackPromptIds.value) | ||
| afterLoading.delete(promptId) | ||
| loadingStackPromptIds.value = afterLoading | ||
|
|
||
| if (!children.length) { | ||
| return | ||
| } | ||
|
|
||
| stackChildrenByPromptId.value = { | ||
| ...stackChildrenByPromptId.value, | ||
| [promptId]: children | ||
| } | ||
| } | ||
|
|
||
| const nextExpanded = new Set(expandedStackPromptIds.value) | ||
| nextExpanded.add(promptId) | ||
| expandedStackPromptIds.value = nextExpanded | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Ensure loading flags clear even on unexpected errors.
If resolveStackChildren throws unexpectedly, the loading set can be left stuck. A try/finally around the await keeps the guard consistent.
♻️ Suggested defensive update
- const children = await resolveStackChildren(asset)
-
- const afterLoading = new Set(loadingStackPromptIds.value)
- afterLoading.delete(promptId)
- loadingStackPromptIds.value = afterLoading
+ let children: AssetItem[] = []
+ try {
+ children = await resolveStackChildren(asset)
+ } finally {
+ const afterLoading = new Set(loadingStackPromptIds.value)
+ afterLoading.delete(promptId)
+ loadingStackPromptIds.value = afterLoading
+ }🤖 Prompt for AI Agents
In `@src/platform/assets/composables/useOutputStacks.ts` around lines 65 - 103,
The toggleStack function can leave loadingStackPromptIds stuck if
resolveStackChildren throws; wrap the await resolveStackChildren(asset) in a
try/finally so the block that removes promptId from loadingStackPromptIds always
runs (move the creation of nextLoading before the try, set loadingStackPromptIds
= nextLoading, await resolveStackChildren inside try, and in finally remove
promptId from loadingStackPromptIds), then proceed to set
stackChildrenByPromptId and expandedStackPromptIds only after a successful
children result; reference toggleStack, resolveStackChildren,
loadingStackPromptIds, stackChildrenByPromptId, expandedStackPromptIds, and
promptId when making the change.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/platform/assets/composables/useOutputStacks.ts`:
- Around line 68-101: The toggleStack flow refetches when children are empty
because you only store non-empty arrays; change it to cache empty results and
check for key existence instead of length. Specifically, in toggleStack use a
presence check on stackChildrenByPromptId.value (e.g.,
Object.prototype.hasOwnProperty.call or promptId in
stackChildrenByPromptId.value) to skip fetching, and after awaiting
resolveStackChildren(asset) always assign stackChildrenByPromptId.value = {
...stackChildrenByPromptId.value, [promptId]: children } (even when
children.length === 0); keep the existing loadingStackPromptIds add/remove logic
around the fetch to prevent concurrent requests. Ensure you still early-return
if loadingStackPromptIds.value.has(promptId) and preserve expandedStackPromptIds
behavior.
In `@src/platform/assets/utils/outputAssetUtil.ts`:
- Around line 40-49: The getOutputKey function treats nodeId with a falsy check
which drops valid numeric ids like 0; update the guard to only treat nodeId as
missing when null or undefined (use nodeId == null) while keeping the existing
falsy checks for subfolder and filename, i.e., change the if condition in
getOutputKey to check nodeId == null || !subfolder || !filename so numeric 0 is
accepted when building the `${nodeId}-${subfolder}-${filename}` key.
Comment on lines
+68
to
+101
| async function toggleStack(asset: AssetItem) { | ||
| const promptId = getStackPromptId(asset) | ||
| if (!promptId) return | ||
|
|
||
| if (expandedStackPromptIds.value.has(promptId)) { | ||
| const next = new Set(expandedStackPromptIds.value) | ||
| next.delete(promptId) | ||
| expandedStackPromptIds.value = next | ||
| return | ||
| } | ||
|
|
||
| if (!stackChildrenByPromptId.value[promptId]?.length) { | ||
| if (loadingStackPromptIds.value.has(promptId)) { | ||
| return | ||
| } | ||
| const nextLoading = new Set(loadingStackPromptIds.value) | ||
| nextLoading.add(promptId) | ||
| loadingStackPromptIds.value = nextLoading | ||
|
|
||
| const children = await resolveStackChildren(asset) | ||
|
|
||
| const afterLoading = new Set(loadingStackPromptIds.value) | ||
| afterLoading.delete(promptId) | ||
| loadingStackPromptIds.value = afterLoading | ||
|
|
||
| if (!children.length) { | ||
| return | ||
| } | ||
|
|
||
| stackChildrenByPromptId.value = { | ||
| ...stackChildrenByPromptId.value, | ||
| [promptId]: children | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Cache “no-children” results to avoid repeated fetches.
Currently, a stack with zero children refetches on every expand attempt because empty results aren’t stored. Cache the empty array and switch the guard to “key exists” rather than “length > 0”.
♻️ Suggested update
- if (!stackChildrenByPromptId.value[promptId]?.length) {
+ if (!Object.hasOwn(stackChildrenByPromptId.value, promptId)) {
if (loadingStackPromptIds.value.has(promptId)) {
return
}
const nextLoading = new Set(loadingStackPromptIds.value)
nextLoading.add(promptId)
loadingStackPromptIds.value = nextLoading
const children = await resolveStackChildren(asset)
const afterLoading = new Set(loadingStackPromptIds.value)
afterLoading.delete(promptId)
loadingStackPromptIds.value = afterLoading
- if (!children.length) {
- return
- }
-
stackChildrenByPromptId.value = {
...stackChildrenByPromptId.value,
[promptId]: children
}
+
+ if (!children.length) {
+ return
+ }
}🤖 Prompt for AI Agents
In `@src/platform/assets/composables/useOutputStacks.ts` around lines 68 - 101,
The toggleStack flow refetches when children are empty because you only store
non-empty arrays; change it to cache empty results and check for key existence
instead of length. Specifically, in toggleStack use a presence check on
stackChildrenByPromptId.value (e.g., Object.prototype.hasOwnProperty.call or
promptId in stackChildrenByPromptId.value) to skip fetching, and after awaiting
resolveStackChildren(asset) always assign stackChildrenByPromptId.value = {
...stackChildrenByPromptId.value, [promptId]: children } (even when
children.length === 0); keep the existing loadingStackPromptIds add/remove logic
around the fetch to prevent concurrent requests. Ensure you still early-return
if loadingStackPromptIds.value.has(promptId) and preserve expandedStackPromptIds
behavior.
Co-authored-by: Johnpaul Chiwetelu <49923152+Myestery@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/sidebar/tabs/AssetsSidebarTab.vue (1)
428-439:⚠️ Potential issue | 🟠 MajorReset gallery index when the active asset disappears.
If a stack collapses (or filtering removes the current asset),
currentGalleryAssetIdwon’t be found and the gallery can show a different item at the old index. Consider resetting the index when the asset is missing to keep the gallery aligned.🛠️ Suggested fix
if (newIndex !== -1) { galleryActiveIndex.value = newIndex + } else { + galleryActiveIndex.value = -1 }
🤖 Fix all issues with AI agents
In `@src/platform/assets/composables/useAssetSelection.test.ts`:
- Around line 47-65: The test's AssetItem mocks used in
describe('reconcileSelection') are missing required fields; update the inline
asset objects (used with useAssetSelection(), useAssetSelectionStore(), and
selection.reconcileSelection([...])) to either include the required properties
(size, created_at, preview_url) or explicitly cast them as partials using the
pattern as Partial<AssetItem> as AssetItem so the test types remain correct and
resilient to schema changes.
Comment on lines
+47
to
+65
| describe('reconcileSelection', () => { | ||
| it('prunes selection to visible assets', () => { | ||
| const selection = useAssetSelection() | ||
| const store = useAssetSelectionStore() | ||
| const assets: AssetItem[] = [ | ||
| { id: 'a', name: 'a.png', tags: [] }, | ||
| { id: 'b', name: 'b.png', tags: [] } | ||
| ] | ||
|
|
||
| store.setSelection(['a', 'b']) | ||
| store.setLastSelectedIndex(1) | ||
| store.setLastSelectedAssetId('b') | ||
|
|
||
| selection.reconcileSelection([assets[1]]) | ||
|
|
||
| expect(Array.from(store.selectedAssetIds)).toEqual(['b']) | ||
| expect(store.lastSelectedIndex).toBe(0) | ||
| expect(store.lastSelectedAssetId).toBe('b') | ||
| }) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using more complete mock objects or explicit partial casting.
The inline AssetItem objects are missing required fields (size, created_at, preview_url) defined in the schema. While this works because TypeScript doesn't enforce runtime shapes, using as Partial<AssetItem> as AssetItem makes the incomplete mock explicit and prevents type drift if the schema gains stricter validation.
♻️ Example for explicit partial casting
const assets: AssetItem[] = [
- { id: 'a', name: 'a.png', tags: [] },
- { id: 'b', name: 'b.png', tags: [] }
+ { id: 'a', name: 'a.png', tags: [] } as Partial<AssetItem> as AssetItem,
+ { id: 'b', name: 'b.png', tags: [] } as Partial<AssetItem> as AssetItem
]Based on learnings: "In test files, when you create mock objects that partially implement an interface, prefer casting with as Partial<InterfaceType> as InterfaceType rather than as any or as unknown as InterfaceType."
📝 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
| describe('reconcileSelection', () => { | |
| it('prunes selection to visible assets', () => { | |
| const selection = useAssetSelection() | |
| const store = useAssetSelectionStore() | |
| const assets: AssetItem[] = [ | |
| { id: 'a', name: 'a.png', tags: [] }, | |
| { id: 'b', name: 'b.png', tags: [] } | |
| ] | |
| store.setSelection(['a', 'b']) | |
| store.setLastSelectedIndex(1) | |
| store.setLastSelectedAssetId('b') | |
| selection.reconcileSelection([assets[1]]) | |
| expect(Array.from(store.selectedAssetIds)).toEqual(['b']) | |
| expect(store.lastSelectedIndex).toBe(0) | |
| expect(store.lastSelectedAssetId).toBe('b') | |
| }) | |
| describe('reconcileSelection', () => { | |
| it('prunes selection to visible assets', () => { | |
| const selection = useAssetSelection() | |
| const store = useAssetSelectionStore() | |
| const assets: AssetItem[] = [ | |
| { id: 'a', name: 'a.png', tags: [] } as Partial<AssetItem> as AssetItem, | |
| { id: 'b', name: 'b.png', tags: [] } as Partial<AssetItem> as AssetItem | |
| ] | |
| store.setSelection(['a', 'b']) | |
| store.setLastSelectedIndex(1) | |
| store.setLastSelectedAssetId('b') | |
| selection.reconcileSelection([assets[1]]) | |
| expect(Array.from(store.selectedAssetIds)).toEqual(['b']) | |
| expect(store.lastSelectedIndex).toBe(0) | |
| expect(store.lastSelectedAssetId).toBe('b') | |
| }) |
🤖 Prompt for AI Agents
In `@src/platform/assets/composables/useAssetSelection.test.ts` around lines 47 -
65, The test's AssetItem mocks used in describe('reconcileSelection') are
missing required fields; update the inline asset objects (used with
useAssetSelection(), useAssetSelectionStore(), and
selection.reconcileSelection([...])) to either include the required properties
(size, created_at, preview_url) or explicitly cast them as partials using the
pattern as Partial<AssetItem> as AssetItem so the test types remain correct and
resilient to schema changes.
Contributor
|
👀 |
Myestery
approved these changes
Feb 2, 2026
| } | ||
|
|
||
| type OutputKeyParts = { | ||
| nodeId?: string | number | null |
Contributor
There was a problem hiding this comment.
we have a NodeId type we could use here
Member
Author
There was a problem hiding this comment.
Good point, I'll add that in a followup PR
| items.push({ | ||
| id: `${promptId}-${outputKey}`, | ||
| name: output.filename, | ||
| size: 0, |
Contributor
There was a problem hiding this comment.
I'm not sure why the size is hardcoded to 0 here
Member
Author
There was a problem hiding this comment.
It’s hardcoded because the output objects we map (ResultItemImpl) simply don’t carry file size, and the history/job APIs don’t provide it yet. AssetItem.size is optional with a “TBD…provided by history API in the future” note, so we’re effectively using 0 as “unknown” and to keep the shape consistent. Also, the UI only shows size if it’s truthy, so 0 suppresses display.
This was referenced Feb 2, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Add expandable output stacks to the assets list view.
Monolith ver. of #8298 and its children
List view currently collapses multi-output jobs into a single row, which makes sibling outputs easy to miss and causes selection/zoom behavior to drift once items are expanded elsewhere. This change adds a stack toggle to list rows, expands child outputs derived from job data, and keeps list-view selection and gallery navigation aligned with the expanded list. Output mapping and “load full outputs” checks are centralized so folder view and stacks share the same helper, and job-detail parsing now yields previewable outputs for the list view. Asset actions now prefer metadata prompt IDs to support the composite IDs used by stacked outputs.
┆Issue is synchronized with this Notion page by Unito