Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 15 additions & 5 deletions src/platform/assets/components/UploadModelConfirmation.vue
Original file line number Diff line number Diff line change
@@ -1,13 +1,22 @@
<template>
<div class="flex flex-col gap-4 text-sm text-muted-foreground">
<!-- Model Info Section -->
<div class="flex flex-col gap-2">
<p class="m-0">
{{ $t('assetBrowser.modelAssociatedWithLink') }}
</p>
<p class="mt-0 text-base-foreground rounded-lg">
{{ metadata?.filename || metadata?.name }}
</p>
<div
class="flex items-center gap-3 bg-secondary-background p-3 rounded-lg"
>
<img
Copy link
Contributor

Choose a reason for hiding this comment

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

v-if="previewImage"
:src="previewImage"
:alt="metadata?.filename || metadata?.name || 'Model preview'"
class="w-14 h-14 rounded object-cover flex-shrink-0"
/>
<p class="m-0 text-base-foreground">
{{ metadata?.filename || metadata?.name }}
</p>
</div>
Comment on lines +7 to +19
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Preview thumbnail wiring looks good; localize fallback alt text

The preview block and previewImage prop are correctly integrated and handle the optional image well. To comply with the i18n guidelines, consider replacing the hard-coded 'Model preview' fallback in the alt binding with a localized string, e.g.:

:alt="
  metadata?.filename ||
  metadata?.name ||
  $t('assetBrowser.modelPreviewAlt', 'Model preview')
"

and add assetBrowser.modelPreviewAlt to src/locales/en/main.json.

Also applies to: 51-54

🤖 Prompt for AI Agents
In src/platform/assets/components/UploadModelConfirmation.vue around lines 7-19
(and similarly at lines 51-54), replace the hard-coded fallback alt text 'Model
preview' with a localized lookup using $t('assetBrowser.modelPreviewAlt', 'Model
preview') so the alt binding becomes: metadata?.filename || metadata?.name ||
$t('assetBrowser.modelPreviewAlt', 'Model preview'); then add the key
"assetBrowser.modelPreviewAlt": "Model preview" to src/locales/en/main.json to
provide the English default.

</div>

<!-- Model Type Selection -->
Expand Down Expand Up @@ -40,7 +49,8 @@ import { useModelTypes } from '@/platform/assets/composables/useModelTypes'
import type { AssetMetadata } from '@/platform/assets/schemas/assetSchema'

defineProps<{
metadata: AssetMetadata | null
metadata?: AssetMetadata
previewImage?: string
}>()

const modelValue = defineModel<string | undefined>()
Expand Down
2 changes: 2 additions & 0 deletions src/platform/assets/components/UploadModelDialog.vue
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
v-else-if="currentStep === 2"
v-model="selectedModelType"
:metadata="wizardData.metadata"
:preview-image="wizardData.previewImage"
/>

<!-- Step 3: Upload Progress -->
Expand All @@ -23,6 +24,7 @@
:error="uploadError"
:metadata="wizardData.metadata"
:model-type="selectedModelType"
:preview-image="wizardData.previewImage"
/>

<!-- Navigation Footer -->
Expand Down
13 changes: 10 additions & 3 deletions src/platform/assets/components/UploadModelProgress.vue
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,14 @@
</p>

<div
class="flex flex-row items-start p-4 bg-modal-card-background rounded-lg"
class="flex flex-row items-center gap-3 p-4 bg-modal-card-background rounded-lg"
>
<img
v-if="previewImage"
:src="previewImage"
:alt="metadata?.filename || metadata?.name || 'Model preview'"
class="w-14 h-14 rounded object-cover flex-shrink-0"
/>
Comment on lines +28 to +35
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Success preview behavior is good; localize alt fallback

The success-state preview image is correctly wired and optional, and the new previewImage prop is well-scoped. As in the confirmation step, the hard-coded 'Model preview' fallback in the alt binding should be localized, for example:

:alt="
  metadata?.filename ||
  metadata?.name ||
  $t('assetBrowser.modelPreviewAlt', 'Model preview')
"

with assetBrowser.modelPreviewAlt added to src/locales/en/main.json.

Also applies to: 69-75

🤖 Prompt for AI Agents
In src/platform/assets/components/UploadModelProgress.vue around lines 28-35
(and also update the similar occurrence around lines 69-75), the img alt binding
currently falls back to the hard-coded string 'Model preview'; replace that
literal with a localized fallback using the translation helper, e.g.
:alt="metadata?.filename || metadata?.name || $t('assetBrowser.modelPreviewAlt',
'Model preview')", and add the key "assetBrowser.modelPreviewAlt": "Model
preview" to src/locales/en/main.json so the fallback is localized.

<div class="flex flex-col justify-center items-start gap-1 flex-1">
<p class="text-base-foreground m-0">
{{ metadata?.filename || metadata?.name }}
Expand Down Expand Up @@ -63,7 +69,8 @@ import type { AssetMetadata } from '@/platform/assets/schemas/assetSchema'
defineProps<{
status: 'idle' | 'uploading' | 'success' | 'error'
error?: string
metadata: AssetMetadata | null
modelType: string | undefined
metadata?: AssetMetadata
modelType?: string
previewImage?: string
}>()
</script>
38 changes: 35 additions & 3 deletions src/platform/assets/composables/useUploadModelWizard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ import { useModelToNodeStore } from '@/stores/modelToNodeStore'

interface WizardData {
url: string
metadata: AssetMetadata | null
metadata?: AssetMetadata
name: string
tags: string[]
previewImage?: string
}

interface ModelTypeOption {
Expand All @@ -30,7 +31,6 @@ export function useUploadModelWizard(modelTypes: Ref<ModelTypeOption[]>) {

const wizardData = ref<WizardData>({
url: '',
metadata: null,
name: '',
tags: []
})
Expand Down Expand Up @@ -91,6 +91,9 @@ export function useUploadModelWizard(modelTypes: Ref<ModelTypeOption[]>) {
// Pre-fill name from metadata
wizardData.value.name = metadata.filename || metadata.name || ''

// Store preview image if available
wizardData.value.previewImage = metadata.preview_image

// Pre-fill model type from metadata tags if available
if (metadata.tags && metadata.tags.length > 0) {
wizardData.value.tags = metadata.tags
Expand Down Expand Up @@ -134,6 +137,34 @@ export function useUploadModelWizard(modelTypes: Ref<ModelTypeOption[]>) {
wizardData.value.metadata?.name ||
'model'

let previewId: string | undefined

// Upload preview image first if available
if (wizardData.value.previewImage) {
try {
const baseFilename = filename.split('.')[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider more robust extension stripping.

split('.')[0] only captures text before the first dot, so my.model.safetensors becomes my. Consider using filename.replace(/\.[^.]+$/, '') or finding the last dot index to preserve the full base name.

Apply this diff to improve base filename extraction:

-        const baseFilename = filename.split('.')[0]
+        const lastDotIndex = filename.lastIndexOf('.')
+        const baseFilename = lastDotIndex !== -1 ? filename.slice(0, lastDotIndex) : filename
📝 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
const baseFilename = filename.split('.')[0]
const lastDotIndex = filename.lastIndexOf('.')
const baseFilename = lastDotIndex !== -1 ? filename.slice(0, lastDotIndex) : filename
🤖 Prompt for AI Agents
In src/platform/assets/composables/useUploadModelWizard.ts around line 147, the
current extraction const baseFilename = filename.split('.')[0] only keeps text
before the first dot and breaks names like "my.model.safetensors"; replace it
with a robust method such as using filename.replace(/\.[^.]+$/, '') or using
lastDot = filename.lastIndexOf('.') and taking filename.substring(0, lastDot ===
-1 ? filename.length : lastDot) so the full base name is preserved while still
stripping only the final extension.


// Extract extension from data URL MIME type
let extension = 'png'
const mimeMatch = wizardData.value.previewImage.match(
/^data:image\/([^;]+);/
)
if (mimeMatch) {
extension = mimeMatch[1] === 'jpeg' ? 'jpg' : mimeMatch[1]
}

const previewAsset = await assetService.uploadAssetFromBase64({
data: wizardData.value.previewImage,
name: `${baseFilename}_preview.${extension}`,
tags: ['preview']
})
previewId = previewAsset.id
} catch (error) {
console.error('Failed to upload preview image:', error)
// Continue with model upload even if preview fails
}
}

await assetService.uploadAssetFromUrl({
url: wizardData.value.url,
name: filename,
Expand All @@ -142,7 +173,8 @@ export function useUploadModelWizard(modelTypes: Ref<ModelTypeOption[]>) {
source: 'civitai',
source_url: wizardData.value.url,
model_type: selectedModelType.value
}
},
preview_id: previewId
})

uploadStatus.value = 'success'
Expand Down
1 change: 1 addition & 0 deletions src/platform/assets/schemas/assetSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const zAssetMetadata = z.object({
name: z.string().optional(),
tags: z.array(z.string()).optional(),
preview_url: z.string().optional(),
preview_image: z.string().optional(),
validation: zValidationResult.optional()
})

Expand Down
56 changes: 55 additions & 1 deletion src/platform/assets/services/assetService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,59 @@ function createAssetService() {
return await res.json()
}

/**
* Uploads an asset from base64 data
*
* @param params - Upload parameters
* @param params.data - Base64 data URL (e.g., "data:image/png;base64,...")
* @param params.name - Display name (determines extension)
* @param params.tags - Optional freeform tags
* @param params.user_metadata - Optional custom metadata object
* @returns Promise<AssetItem & { created_new: boolean }> - Asset object with created_new flag
* @throws Error if upload fails
*/
async function uploadAssetFromBase64(params: {
data: string
name: string
tags?: string[]
user_metadata?: Record<string, any>
}): Promise<AssetItem & { created_new: boolean }> {
// Validate that data is a data URL
if (!params.data || !params.data.startsWith('data:')) {
throw new Error(
'Invalid data URL: expected a string starting with "data:"'
)
}

// Convert base64 data URL to Blob
const blob = await fetch(params.data).then((r) => r.blob())

// Create FormData and append the blob
const formData = new FormData()
formData.append('file', blob, params.name)

if (params.tags) {
formData.append('tags', JSON.stringify(params.tags))
}

if (params.user_metadata) {
formData.append('user_metadata', JSON.stringify(params.user_metadata))
}

const res = await api.fetchApi(ASSETS_ENDPOINT, {
method: 'POST',
body: formData
})

if (!res.ok) {
throw new Error(
`Failed to upload asset from base64: ${res.status} ${res.statusText}`
)
}

return await res.json()
}
Comment on lines +406 to +446
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 9, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

uploadAssetFromBase64 implementation is solid; align error handling & i18n with uploadAssetFromUrl

The data URL guard, Blob conversion, and FormData upload look correct. To keep UX and localization consistent with uploadAssetFromUrl, consider reusing the same localized error message (assetBrowser.errorUploadFailed) instead of hard-coded English strings and interpolated status text:

   async function uploadAssetFromBase64(params: {
     data: string
     name: string
     tags?: string[]
     user_metadata?: Record<string, any>
   }): Promise<AssetItem & { created_new: boolean }> {
-    // Validate that data is a data URL
-    if (!params.data || !params.data.startsWith('data:')) {
-      throw new Error(
-        'Invalid data URL: expected a string starting with "data:"'
-      )
-    }
+    // Validate that data is a data URL
+    if (!params.data || !params.data.startsWith('data:')) {
+      throw new Error(
+        st(
+          'assetBrowser.errorUploadFailed',
+          'Failed to upload asset. Please try again.'
+        )
+      )
+    }
@@
-    const res = await api.fetchApi(ASSETS_ENDPOINT, {
+    const res = await api.fetchApi(ASSETS_ENDPOINT, {
       method: 'POST',
       body: formData
     })
 
-    if (!res.ok) {
-      throw new Error(
-        `Failed to upload asset from base64: ${res.status} ${res.statusText}`
-      )
-    }
+    if (!res.ok) {
+      throw new Error(
+        st(
+          'assetBrowser.errorUploadFailed',
+          'Failed to upload asset. Please try again.'
+        )
+      )
+    }
 
     return await res.json()
   }

This keeps failure behavior consistent across both upload paths and avoids leaking raw status text to end users while respecting the existing i18n pattern.

📝 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
async function uploadAssetFromBase64(params: {
data: string
name: string
tags?: string[]
user_metadata?: Record<string, any>
}): Promise<AssetItem & { created_new: boolean }> {
// Validate that data is a data URL
if (!params.data || !params.data.startsWith('data:')) {
throw new Error(
'Invalid data URL: expected a string starting with "data:"'
)
}
// Convert base64 data URL to Blob
const blob = await fetch(params.data).then((r) => r.blob())
// Create FormData and append the blob
const formData = new FormData()
formData.append('file', blob, params.name)
if (params.tags) {
formData.append('tags', JSON.stringify(params.tags))
}
if (params.user_metadata) {
formData.append('user_metadata', JSON.stringify(params.user_metadata))
}
const res = await api.fetchApi(ASSETS_ENDPOINT, {
method: 'POST',
body: formData
})
if (!res.ok) {
throw new Error(
`Failed to upload asset from base64: ${res.status} ${res.statusText}`
)
}
return await res.json()
}
async function uploadAssetFromBase64(params: {
data: string
name: string
tags?: string[]
user_metadata?: Record<string, any>
}): Promise<AssetItem & { created_new: boolean }> {
// Validate that data is a data URL
if (!params.data || !params.data.startsWith('data:')) {
throw new Error(
st(
'assetBrowser.errorUploadFailed',
'Failed to upload asset. Please try again.'
)
)
}
// Convert base64 data URL to Blob
const blob = await fetch(params.data).then((r) => r.blob())
// Create FormData and append the blob
const formData = new FormData()
formData.append('file', blob, params.name)
if (params.tags) {
formData.append('tags', JSON.stringify(params.tags))
}
if (params.user_metadata) {
formData.append('user_metadata', JSON.stringify(params.user_metadata))
}
const res = await api.fetchApi(ASSETS_ENDPOINT, {
method: 'POST',
body: formData
})
if (!res.ok) {
throw new Error(
st(
'assetBrowser.errorUploadFailed',
'Failed to upload asset. Please try again.'
)
)
}
return await res.json()
}
🤖 Prompt for AI Agents
In src/platform/assets/services/assetService.ts around lines 406 to 446, replace
the hard-coded English error thrown on upload failure with the existing i18n
error used by uploadAssetFromUrl (assetBrowser.errorUploadFailed) so user-facing
messages are localized and consistent; do not surface raw HTTP statusText to end
users — if you need the status for diagnostics, include it in a debug/process
log or attach it to an internal error property but throw the localized message
to callers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

these are internal facing errors. The current error messages give us more detail about what actually went wrong

Copy link
Contributor

Choose a reason for hiding this comment

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

@luke-mino-altherr Understood! That makes sense for internal tooling—keeping the detailed status information (${res.status} ${res.statusText}) is valuable for debugging. I'll leave the error messages as-is. 👍


🧠 Learnings used
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: User-friendly and actionable error messages in error handling


return {
getAssetModelFolders,
getAssetModels,
Expand All @@ -402,7 +455,8 @@ function createAssetService() {
deleteAsset,
updateAsset,
getAssetMetadata,
uploadAssetFromUrl
uploadAssetFromUrl,
uploadAssetFromBase64
}
}

Expand Down