fix: support text and misc generated asset states#8914
Conversation
📝 WalkthroughWalkthroughThe changes extend media type support to include "text" and "other" categories alongside existing image, video, audio, and 3D types. This includes updated file detection logic, new UI components for preview display, expanded icon mappings, schema updates, and corresponding test coverage. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
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 |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 02/21/2026, 09:10:56 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
|
Playwright: ✅ 522 passed, 0 failed · 6 flaky 📊 Browser Reports
|
There was a problem hiding this comment.
Pull request overview
Aligns generated-asset media-kind classification across the app by extending the shared filename-based detector and wiring new text/other states through both card and list previews.
Changes:
- Extended shared
getMediaTypeFromFilenameto detect more extensions and to classify unknown/empty asother(plus added tests). - Updated assets list previews to only use
preview_urlforimage/video, and added icon support fortext/other. - Added card-top preview components for
textandother, and widened the frontend media-kind schema accordingly.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/platform/assets/utils/mediaIconUtil.ts | Adds icon mappings for text and other. |
| src/platform/assets/utils/mediaIconUtil.test.ts | Adds coverage for the new icon mappings and verifies existing ones. |
| src/platform/assets/schemas/mediaAssetSchema.ts | Extends MediaKind union to include text and other. |
| src/platform/assets/components/MediaTextTop.vue | New card-top UI for text assets (icon placeholder). |
| src/platform/assets/components/MediaOtherTop.vue | New card-top UI for misc/unknown assets (icon placeholder). |
| src/platform/assets/components/MediaAssetCard.vue | Routes card-top rendering via shared media-type detection and supports new preview kinds. |
| src/components/sidebar/tabs/AssetsSidebarListView.vue | Prevents non-image/video assets from using preview_url and falls back to icons. |
| src/components/sidebar/tabs/AssetsSidebarListView.test.ts | Updates mocks and adds test coverage for icon fallback behavior. |
| packages/shared-frontend-utils/src/formatUtil.ts | Extends extension sets; returns text/other; changes unknown fallback to other. |
| packages/shared-frontend-utils/src/formatUtil.test.ts | Updates expectations and adds tests for new extension handling and fallbacks. |
| <i class="icon-[lucide--text] text-3xl text-base-foreground" /> | ||
| </div> | ||
| </div> | ||
| </template> |
There was a problem hiding this comment.
MediaAssetCard passes an asset prop into this component, but MediaTextTop doesn't declare any props, so Vue will treat asset as a DOM attribute (likely ending up as asset="[object Object]"). Define an asset prop (same AssetMeta type as the other Media*Top components) or disable attribute inheritance to avoid leaking non-HTML attributes into the DOM.
| </template> | |
| </template> | |
| <script setup lang="ts"> | |
| interface Props { | |
| asset?: unknown; | |
| } | |
| const props = defineProps<Props>(); | |
| </script> |
| <i class="icon-[lucide--check-check] text-3xl text-base-foreground" /> | ||
| </div> | ||
| </div> | ||
| </template> |
There was a problem hiding this comment.
MediaAssetCard passes an asset prop into this component, but MediaOtherTop doesn't declare any props, so Vue will treat asset as a DOM attribute (likely ending up as asset="[object Object]"). Define an asset prop (same AssetMeta type as the other Media*Top components) or disable attribute inheritance to avoid leaking non-HTML attributes into the DOM.
| </template> | |
| </template> | |
| <script setup lang="ts"> | |
| defineOptions({ | |
| inheritAttrs: false, | |
| }); | |
| </script> |
| }) | ||
|
|
||
| const previewKind = computed((): PreviewKind => { | ||
| return getMediaTypeFromFilename(asset?.name || '') |
There was a problem hiding this comment.
fileKind and previewKind are computed from the same getMediaTypeFromFilename(asset?.name || '') expression, which duplicates work and can drift if one is updated later. Consider using a single computed (or reusing fileKind) for both the top-component selection and the legacy adaptedAsset.kind field.
| return getMediaTypeFromFilename(asset?.name || '') | |
| return fileKind.value as PreviewKind |
| case 'other': | ||
| return 'icon-[lucide--check-check]' | ||
| default: | ||
| return 'icon-[lucide--image]' |
There was a problem hiding this comment.
Now that unknown/empty filenames are classified as other, the default case here will only be hit for truly unexpected values. Returning the image icon as a fallback is inconsistent with the new other semantics; consider falling back to the other icon instead to avoid showing an image icon for unknown types.
| return 'icon-[lucide--image]' | |
| return 'icon-[lucide--check-check]' |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e21768b0b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (VIDEO_EXTENSIONS.includes(ext as VideoExtension)) return 'video' | ||
| if (AUDIO_EXTENSIONS.includes(ext as AudioExtension)) return 'audio' | ||
| if (THREE_D_EXTENSIONS.includes(ext as ThreeDExtension)) return '3D' | ||
| if (TEXT_EXTENSIONS.includes(ext as TextExtension)) return 'text' |
There was a problem hiding this comment.
Block inspect for non-previewable text/other assets
Returning text here causes .txt/.md/.json/... assets to take the standard inspect/zoom path, but handleZoomClick in AssetsSidebarTab.vue only special-cases 3D and ResultGallery.vue only renders isImage, isVideo, or isAudio items. In practice, inspecting one of these newly classified text/other assets opens a fullscreen gallery with no rendered content. Please disable inspect/zoom for non-previewable kinds (or add a renderer) so these assets do not open a blank viewer.
Useful? React with 👍 / 👎.
9d99871 to
2a8084c
Compare
213bae6 to
79ee412
Compare
…bar-tab-scaffold # Conflicts: # src/components/queue/QueueOverlayHeader.vue # src/components/sidebar/tabs/AssetsSidebarTab.vue # src/components/ui/Popover.vue
## Summary Fix 3D inspect dialog rendering by wrapping `Load3dViewerContent` with `defineAsyncComponent` before passing it to `dialogStore.showDialog`. ## Changes - Update `AssetsSidebarTab` to lazy-load `Load3dViewerContent` via `defineAsyncComponent(() => import(...))`. ## Why Passing a raw `() => import(...)` loader to the dialog store can render `[object Promise]` instead of the component in this path. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8990-fix-load-3D-dialog-content-with-defineAsyncComponent-30c6d73d365081e7ba82cc8282943756) by [Unito](https://www.unito.io)
8d2488a to
c5a4ab5
Compare
98e2789 to
f961291
Compare
Merge activity
|
The base branch was changed.
…double-check-fallback # Conflicts: # src/components/sidebar/tabs/AssetsSidebarListView.test.ts # src/components/sidebar/tabs/AssetsSidebarListView.vue # src/platform/assets/components/MediaAssetFilterBar.vue # src/platform/assets/components/MediaVideoTop.test.ts
📦 Bundle: 4.37 MB gzip 🔴 +1.19 kBDetailsSummary
Category Glance App Entry Points — 21.5 kB (baseline 21.5 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 942 kB (baseline 942 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 68.8 kB (baseline 68.8 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 11 added / 11 removed Panels & Settings — 436 kB (baseline 436 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 6 added / 6 removed Editors & Dialogs — 738 B (baseline 738 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 43.2 kB (baseline 43.2 kB) • ⚪ 0 BReusable component library chunks
Status: 8 added / 8 removed Data & Services — 2.51 MB (baseline 2.51 MB) • 🔴 +990 BStores, services, APIs, and repositories
Status: 14 added / 14 removed Utilities & Hooks — 57.7 kB (baseline 57.7 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 12 added / 12 removed Vendor & Third-Party — 8.86 MB (baseline 8.86 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.6 MB (baseline 7.6 MB) • 🔴 +2.02 kBBundles that do not match a named category
Status: 63 added / 61 removed |
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/platform/assets/components/MediaAssetCard.vue (1)
38-49:⚠️ Potential issue | 🟡 MinorAdd proper props and emits declarations to
MediaTextTop.vueandMediaOtherTop.vue, or remove unused prop/event bindings from the parent.
MediaTextTop.vueandMediaOtherTop.vueare template-only components without prop or emit declarations, yet the parent passes:asset,:context, and several events including@view,@download,@video-playing-state-changed,@video-controls-changed, and@image-loaded. Since these components don't use any of these props or emit any of these events, either:
- Add explicit
definePropsanddefineEmitsdeclarations (even if empty) for documentation and type safety, or- Remove the unused
:contextbinding and event listeners from the parent componentNote:
contextis already provided to child components viaprovide()at line 243–248, making the prop binding redundant. Additionally, no component in the component hierarchy emits a@downloadevent, so that listener should be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/assets/components/MediaAssetCard.vue` around lines 38 - 49, The parent MediaAssetCard.vue is passing :asset, :context and several event listeners to components returned by getTopComponent(previewKind) (used when adaptedAsset exists and the template binding mounts the component) but MediaTextTop.vue and MediaOtherTop.vue are template-only and lack defineProps/defineEmits; either add explicit defineProps({ asset: Object, context: Object? }) and defineEmits(['view','download','video-playing-state-changed','video-controls-changed','image-loaded']) to MediaTextTop.vue and MediaOtherTop.vue to document and type these bindings, or remove the redundant bindings from MediaAssetCard.vue—specifically drop :context (it's provided via provide()), remove the `@download` listener (no emitter in hierarchy), and remove any unused `@video-playing-state-changed`, `@video-controls-changed`, and `@image-loaded` handlers if those children don’t emit them; keep handleZoomClick, actions.downloadAsset(), isVideoPlaying, showVideoControls, and handleImageLoaded only if corresponding emits/props exist.
🧹 Nitpick comments (4)
packages/shared-frontend-utils/src/formatUtil.test.ts (1)
104-112: Missing test coverage foryml,xml,log,markdowninTEXT_EXTENSIONSThe
TEXT_EXTENSIONSconstant includesyml,xml,log, andmarkdown, but none of these are covered by the text-file test group. Adding them closes a gap in case a future change accidentally removes them.✅ Suggested additions
it('should identify text file extensions correctly', () => { expect(getMediaTypeFromFilename('notes.txt')).toBe('text') expect(getMediaTypeFromFilename('readme.md')).toBe('text') expect(getMediaTypeFromFilename('data.json')).toBe('text') expect(getMediaTypeFromFilename('table.csv')).toBe('text') expect(getMediaTypeFromFilename('config.yaml')).toBe('text') + expect(getMediaTypeFromFilename('config.yml')).toBe('text') + expect(getMediaTypeFromFilename('schema.xml')).toBe('text') + expect(getMediaTypeFromFilename('debug.log')).toBe('text') + expect(getMediaTypeFromFilename('notes.markdown')).toBe('text') })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared-frontend-utils/src/formatUtil.test.ts` around lines 104 - 112, The test suite in formatUtil.test.ts is missing coverage for TEXT_EXTENSIONS entries 'yml', 'xml', 'log', and 'markdown'; update the 'text files' test case that calls getMediaTypeFromFilename to include filenames exercising those extensions (e.g., 'config.yml', 'schema.xml', 'app.log', 'notes.markdown' or '.md' variant) so assertions expect 'text' for each; ensure you reference the same getMediaTypeFromFilename helper used in the file and keep assertions consistent with existing ones to prevent regressions if TEXT_EXTENSIONS is changed.src/platform/assets/components/MediaAssetCard.vue (2)
155-155:PreviewKindmay drift fromMediaKind— verify alignment.
PreviewKindis derived fromReturnType<typeof getMediaTypeFromFilename>, whileMediaKindis derived from the Zod schema. IfgetMediaTypeFromFilenameever returns a value not inMediaKind(or vice versa), these types will silently diverge, potentially causing type errors inadaptedAsset.kind(line 230) which expectsMediaKind.Consider deriving
PreviewKindfromMediaKinddirectly, or adding a compile-time assertion that the two types are assignable to each other.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/assets/components/MediaAssetCard.vue` at line 155, PreviewKind currently comes from ReturnType<typeof getMediaTypeFromFilename> and can drift from the Zod-derived MediaKind, causing mismatches when assigning to adaptedAsset.kind; fix by making PreviewKind derived from MediaKind (replace the ReturnType-based alias with one that references MediaKind) or add a compile-time mutual-assignability assertion between PreviewKind and MediaKind (use a conditional type check that ensures PreviewKind extends MediaKind and MediaKind extends PreviewKind) so the compiler will fail if they diverge; update references to PreviewKind and ensure getMediaTypeFromFilename’s return type is narrowed/annotated to match MediaKind if needed.
212-218:fileKindandpreviewKindare computed identically — consolidate to remove redundant reactive binding.Both computed properties call
getMediaTypeFromFilename(asset?.name || '')and return structurally equivalent types (MediaKindandPreviewKindboth resolve to'image' | 'video' | 'audio' | '3D' | 'text' | 'other'). This creates two separate reactive subscriptions for the same derivation, which is unnecessary.Suggested consolidation
-const fileKind = computed((): MediaKind => { - return getMediaTypeFromFilename(asset?.name || '') -}) - -const previewKind = computed((): PreviewKind => { - return getMediaTypeFromFilename(asset?.name || '') -}) +const fileKind = computed((): PreviewKind => { + return getMediaTypeFromFilename(asset?.name || '') +})Then use
fileKindeverywhere (including line 39's:is="getTopComponent(fileKind)").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/assets/components/MediaAssetCard.vue` around lines 212 - 218, Consolidate the duplicate computed properties by removing previewKind and reusing fileKind (computed via getMediaTypeFromFilename(asset?.name || '')) wherever previewKind was used; update references such as the :is binding that calls getTopComponent(previewKind) to use getTopComponent(fileKind) and ensure the fileKind return type remains compatible with both MediaKind and PreviewKind usages so you only maintain one reactive subscription.src/platform/assets/components/MediaOtherTop.vue (1)
1-9: Props from the parent will fall through as HTML attributes on the root<div>.The parent (
MediaAssetCard.vue) binds:asset,:context, and several event listeners to whichever<component :is>is rendered. Since this is a template-only component with no props/emits defined, Vue's fallthrough attrs will attach them to the root<div>, producing meaninglessasset="[object Object]"attributes in the DOM.Consider adding a minimal script block to opt out:
Suggested fix
+<script setup lang="ts"> +defineOptions({ inheritAttrs: false }) +</script> + <template> <div class="relative size-full overflow-hidden rounded">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/assets/components/MediaOtherTop.vue` around lines 1 - 9, This template-only component (MediaOtherTop.vue) is receiving parent props/events as DOM attributes on the root <div> (from MediaAssetCard.vue); add a minimal script to opt out of attribute fallthrough—either add a classic script with export default { inheritAttrs: false } or in script setup call defineOptions({ inheritAttrs: false })—so props like :asset and :context and bound events are not attached as meaningless attributes to the root element.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/sidebar/tabs/AssetsSidebarListView.vue`:
- Around line 145-151: getAssetPreviewUrl currently only returns preview_url for
mediaTypes 'image' and 'video', which discards valid previews for audio and 3D
assets; update the function getAssetPreviewUrl to also return asset.preview_url
when getAssetMediaType(asset) returns 'audio' or '3d' (or any other types that
may carry previews) so that waveform thumbnails or 3D render thumbnails are
honoured; ensure you reference the AssetItem.preview_url property and
getAssetMediaType(asset) and keep existing fallback to '' when preview_url is
absent.
---
Outside diff comments:
In `@src/platform/assets/components/MediaAssetCard.vue`:
- Around line 38-49: The parent MediaAssetCard.vue is passing :asset, :context
and several event listeners to components returned by
getTopComponent(previewKind) (used when adaptedAsset exists and the template
binding mounts the component) but MediaTextTop.vue and MediaOtherTop.vue are
template-only and lack defineProps/defineEmits; either add explicit
defineProps({ asset: Object, context: Object? }) and
defineEmits(['view','download','video-playing-state-changed','video-controls-changed','image-loaded'])
to MediaTextTop.vue and MediaOtherTop.vue to document and type these bindings,
or remove the redundant bindings from MediaAssetCard.vue—specifically drop
:context (it's provided via provide()), remove the `@download` listener (no
emitter in hierarchy), and remove any unused `@video-playing-state-changed`,
`@video-controls-changed`, and `@image-loaded` handlers if those children don’t emit
them; keep handleZoomClick, actions.downloadAsset(), isVideoPlaying,
showVideoControls, and handleImageLoaded only if corresponding emits/props
exist.
---
Nitpick comments:
In `@packages/shared-frontend-utils/src/formatUtil.test.ts`:
- Around line 104-112: The test suite in formatUtil.test.ts is missing coverage
for TEXT_EXTENSIONS entries 'yml', 'xml', 'log', and 'markdown'; update the
'text files' test case that calls getMediaTypeFromFilename to include filenames
exercising those extensions (e.g., 'config.yml', 'schema.xml', 'app.log',
'notes.markdown' or '.md' variant) so assertions expect 'text' for each; ensure
you reference the same getMediaTypeFromFilename helper used in the file and keep
assertions consistent with existing ones to prevent regressions if
TEXT_EXTENSIONS is changed.
In `@src/platform/assets/components/MediaAssetCard.vue`:
- Line 155: PreviewKind currently comes from ReturnType<typeof
getMediaTypeFromFilename> and can drift from the Zod-derived MediaKind, causing
mismatches when assigning to adaptedAsset.kind; fix by making PreviewKind
derived from MediaKind (replace the ReturnType-based alias with one that
references MediaKind) or add a compile-time mutual-assignability assertion
between PreviewKind and MediaKind (use a conditional type check that ensures
PreviewKind extends MediaKind and MediaKind extends PreviewKind) so the compiler
will fail if they diverge; update references to PreviewKind and ensure
getMediaTypeFromFilename’s return type is narrowed/annotated to match MediaKind
if needed.
- Around line 212-218: Consolidate the duplicate computed properties by removing
previewKind and reusing fileKind (computed via
getMediaTypeFromFilename(asset?.name || '')) wherever previewKind was used;
update references such as the :is binding that calls
getTopComponent(previewKind) to use getTopComponent(fileKind) and ensure the
fileKind return type remains compatible with both MediaKind and PreviewKind
usages so you only maintain one reactive subscription.
In `@src/platform/assets/components/MediaOtherTop.vue`:
- Around line 1-9: This template-only component (MediaOtherTop.vue) is receiving
parent props/events as DOM attributes on the root <div> (from
MediaAssetCard.vue); add a minimal script to opt out of attribute
fallthrough—either add a classic script with export default { inheritAttrs:
false } or in script setup call defineOptions({ inheritAttrs: false })—so props
like :asset and :context and bound events are not attached as meaningless
attributes to the root element.
| function getAssetPreviewUrl(asset: AssetItem): string { | ||
| const mediaType = getAssetMediaType(asset) | ||
| if (mediaType === 'image' || mediaType === 'video') { | ||
| return asset.preview_url || '' | ||
| } | ||
| return '' | ||
| } |
There was a problem hiding this comment.
preview_url suppressed for audio and 3D assets too, not just text/other
The PR description says only text/other use icon fallback, but getAssetPreviewUrl returns '' for all non-image/video types — audio and 3D included. If either media type ever carries a preview_url (e.g., a waveform thumbnail for audio, a rendered scene thumbnail for 3D), it will be silently discarded in the list view.
Verify this is intentional, and if so, consider updating the PR description to reflect the full scope of the change. If audio/3D previews should be honoured:
🔧 Narrowed fix if audio/3D should keep preview_url
function getAssetPreviewUrl(asset: AssetItem): string {
const mediaType = getAssetMediaType(asset)
- if (mediaType === 'image' || mediaType === 'video') {
+ if (mediaType === 'image' || mediaType === 'video' || mediaType === 'audio' || mediaType === '3D') {
return asset.preview_url || ''
}
return ''
}📝 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.
| function getAssetPreviewUrl(asset: AssetItem): string { | |
| const mediaType = getAssetMediaType(asset) | |
| if (mediaType === 'image' || mediaType === 'video') { | |
| return asset.preview_url || '' | |
| } | |
| return '' | |
| } | |
| function getAssetPreviewUrl(asset: AssetItem): string { | |
| const mediaType = getAssetMediaType(asset) | |
| if (mediaType === 'image' || mediaType === 'video' || mediaType === 'audio' || mediaType === '3D') { | |
| return asset.preview_url || '' | |
| } | |
| return '' | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/sidebar/tabs/AssetsSidebarListView.vue` around lines 145 -
151, getAssetPreviewUrl currently only returns preview_url for mediaTypes
'image' and 'video', which discards valid previews for audio and 3D assets;
update the function getAssetPreviewUrl to also return asset.preview_url when
getAssetMediaType(asset) returns 'audio' or '3d' (or any other types that may
carry previews) so that waveform thumbnails or 3D render thumbnails are
honoured; ensure you reference the AssetItem.preview_url property and
getAssetMediaType(asset) and keep existing fallback to '' when preview_url is
absent.

Summary
Align generated-asset state classification to a single shared source and implement the missing text/misc states in both card and list previews.
Changes
getMediaTypeFromFilenameinpackages/shared-frontend-utilsto returntextandother, and changed unknown/no-extension fallback fromimagetoother.txt,md,json,csv,yaml/yml,xml,log) and kept existing media kinds.text->icon-[lucide--text]other->icon-[lucide--check-check]image/videouse preview media URLs;text/otheruse icon fallback.textandother.Review Focus
Screenshots (if applicable)
┆Issue is synchronized with this Notion page by Unito