Merge pull request #6477 from menloresearch/fix/valdidate-mmproj

fix: validate mmproj from general basename

web-app/src/containers/dialogs/ImportVisionModelDialog.tsx

@@ -10,7 +10,7 @@ import {
 import { Button } from '@/components/ui/button'
 import { Switch } from '@/components/ui/switch'
 import { useServiceHub } from '@/hooks/useServiceHub'
-import { useState } from 'react'
+import { useState, useEffect, useCallback } from 'react'
 import { toast } from 'sonner'
 import {
   IconLoader2,
@@ -44,7 +44,7 @@ export const ImportVisionModelDialog = ({
   >(null)
   const [isValidatingMmproj, setIsValidatingMmproj] = useState(false)
 
-  const validateGgufFile = async (
+  const validateGgufFile = useCallback(async (
     filePath: string,
     fileType: 'model' | 'mmproj'
   ): Promise<void> => {
@@ -57,8 +57,6 @@ export const ImportVisionModelDialog = ({
     }
 
     try {
-      console.log(`Reading GGUF metadata for ${fileType}:`, filePath)
-
       // Handle validation differently for model files vs mmproj files
       if (fileType === 'model') {
         // For model files, use the standard validateGgufFile method
@@ -66,16 +64,16 @@ export const ImportVisionModelDialog = ({
           const result = await serviceHub.models().validateGgufFile(filePath)
 
           if (result.metadata) {
-            // Log full metadata for debugging
-            console.log(
-              `Full GGUF metadata for ${fileType}:`,
-              JSON.stringify(result.metadata, null, 2)
-            )
-
             // Check architecture from metadata
             const architecture =
               result.metadata.metadata?.['general.architecture']
-            console.log(`${fileType} architecture:`, architecture)
+
+            // Extract baseName and use it as model name if available
+            const baseName = result.metadata.metadata?.['general.basename']
+
+            if (baseName) {
+              setModelName(baseName)
+            }
 
             // Model files should NOT be clip
             if (architecture === 'clip') {
@@ -86,11 +84,6 @@ export const ImportVisionModelDialog = ({
                 'CLIP architecture detected in model file:',
                 architecture
               )
-            } else {
-              console.log(
-                'Model validation passed. Architecture:',
-                architecture
-              )
             }
           }
 
@@ -109,16 +102,15 @@ export const ImportVisionModelDialog = ({
             path: filePath,
           })
 
-          console.log(
-            `Full GGUF metadata for ${fileType}:`,
-            JSON.stringify(metadata, null, 2)
-          )
 
           // Check if architecture matches expected type
           const architecture = (
             metadata as { metadata?: Record<string, string> }
           ).metadata?.['general.architecture']
-          console.log(`${fileType} architecture:`, architecture)
+
+          // Get general.baseName from metadata
+          const baseName = (metadata as { metadata?: Record<string, string> })
+            .metadata?.['general.basename']
 
           // MMProj files MUST be clip
           if (architecture !== 'clip') {
@@ -128,11 +120,19 @@ export const ImportVisionModelDialog = ({
               'Non-CLIP architecture detected in mmproj file:',
               architecture
             )
-          } else {
+          } else if (
-            console.log(
+            baseName &&
-              'MMProj validation passed. Architecture:',
+            modelName &&
-              architecture
+            !modelName.toLowerCase().includes(baseName.toLowerCase()) &&
-            )
+            !baseName.toLowerCase().includes(modelName.toLowerCase())
+          ) {
+            // Validate that baseName and model name are compatible (one should contain the other)
+            const errorMessage = `MMProj file baseName "${baseName}" does not match model name "${modelName}". The MMProj file should be compatible with the selected model.`
+            setMmprojValidationError(errorMessage)
+            console.error('BaseName mismatch in mmproj file:', {
+              baseName,
+              modelName,
+            })
           }
         } catch (directError) {
           console.error('Failed to validate mmproj file directly:', directError)
@@ -158,15 +158,15 @@ export const ImportVisionModelDialog = ({
         setIsValidatingMmproj(false)
       }
     }
-  }
+  }, [modelName, serviceHub])
 
-  const validateModelFile = async (filePath: string): Promise<void> => {
+  const validateModelFile = useCallback(async (filePath: string): Promise<void> => {
     await validateGgufFile(filePath, 'model')
-  }
+  }, [validateGgufFile])
 
-  const validateMmprojFile = async (filePath: string): Promise<void> => {
+  const validateMmprojFile = useCallback(async (filePath: string): Promise<void> => {
     await validateGgufFile(filePath, 'mmproj')
-  }
+  }, [validateGgufFile])
 
   const handleFileSelect = async (type: 'model' | 'mmproj') => {
     const selectedFile = await serviceHub.dialog().open({
@@ -179,14 +179,14 @@ export const ImportVisionModelDialog = ({
 
       if (type === 'model') {
         setModelFile(selectedFile)
-        // Auto-generate model name from GGUF file
+        // Set temporary model name from filename (will be overridden by baseName from metadata if available)
         const sanitizedName = fileName
           .replace(/\s/g, '-')
           .replace(/\.(gguf|GGUF)$/, '')
           .replace(/[^a-zA-Z0-9/_.-]/g, '') // Remove any characters not allowed in model IDs
         setModelName(sanitizedName)
 
-        // Validate the selected model file
+        // Validate the selected model file (this will update model name with baseName from metadata)
         await validateModelFile(selectedFile)
       } else {
         setMmProjFile(selectedFile)
@@ -272,6 +272,13 @@ export const ImportVisionModelDialog = ({
     setIsValidatingMmproj(false)
   }
 
+  // Re-validate MMProj file when model name changes
+  useEffect(() => {
+    if (mmProjFile && modelName && isVisionModel) {
+      validateMmprojFile(mmProjFile)
+    }
+  }, [modelName, mmProjFile, isVisionModel, validateMmprojFile])
+
   const handleOpenChange = (newOpen: boolean) => {
     if (!importing) {
       setOpen(newOpen)

web-app/src/routes/settings/providers/$providerName.tsx

@@ -83,6 +83,7 @@ function ProviderDetail() {
   const [refreshingModels, setRefreshingModels] = useState(false)
   const [isCheckingBackendUpdate, setIsCheckingBackendUpdate] = useState(false)
   const [isInstallingBackend, setIsInstallingBackend] = useState(false)
+  const [importingModel, setImportingModel] = useState<string | null>(null)
   const { checkForUpdate: checkForBackendUpdate, installBackend } =
     useBackendUpdater()
   const { providerName } = useParams({ from: Route.id })
@@ -102,58 +103,66 @@ function ProviderDetail() {
     )
 
   const handleModelImportSuccess = async (importedModelName?: string) => {
-    // Refresh the provider to update the models list
+    if (importedModelName) {
-    await serviceHub.providers().getProviders().then(setProviders)
+      setImportingModel(importedModelName)
+    }
 
-    // If a model was imported and it might have vision capabilities, check and update
+    try {
-    if (importedModelName && providerName === 'llamacpp') {
+      // Refresh the provider to update the models list
-      try {
+      await serviceHub.providers().getProviders().then(setProviders)
-        const mmprojExists = await serviceHub
-          .models()
-          .checkMmprojExists(importedModelName)
-        if (mmprojExists) {
-          // Get the updated provider after refresh
-          const { getProviderByName, updateProvider: updateProviderState } =
-            useModelProvider.getState()
-          const llamacppProvider = getProviderByName('llamacpp')
 
-          if (llamacppProvider) {
+      // If a model was imported and it might have vision capabilities, check and update
-            const modelIndex = llamacppProvider.models.findIndex(
+      if (importedModelName && providerName === 'llamacpp') {
-              (m: Model) => m.id === importedModelName
+        try {
-            )
+          const mmprojExists = await serviceHub
-            if (modelIndex !== -1) {
+            .models()
-              const model = llamacppProvider.models[modelIndex]
+            .checkMmprojExists(importedModelName)
-              const capabilities = model.capabilities || []
+          if (mmprojExists) {
+            // Get the updated provider after refresh
+            const { getProviderByName, updateProvider: updateProviderState } =
+              useModelProvider.getState()
+            const llamacppProvider = getProviderByName('llamacpp')
 
-              // Add 'vision' capability if not already present AND if user hasn't manually configured capabilities
+            if (llamacppProvider) {
-              // Check if model has a custom capabilities config flag
+              const modelIndex = llamacppProvider.models.findIndex(
+                (m: Model) => m.id === importedModelName
+              )
+              if (modelIndex !== -1) {
+                const model = llamacppProvider.models[modelIndex]
+                const capabilities = model.capabilities || []
 
-              const hasUserConfiguredCapabilities =
+                // Add 'vision' capability if not already present AND if user hasn't manually configured capabilities
-                (model as any)._userConfiguredCapabilities === true
+                // Check if model has a custom capabilities config flag
 
-              if (
+                const hasUserConfiguredCapabilities =
-                !capabilities.includes('vision') &&
+                  (model as any)._userConfiguredCapabilities === true
-                !hasUserConfiguredCapabilities
-              ) {
-                const updatedModels = [...llamacppProvider.models]
-                updatedModels[modelIndex] = {
-                  ...model,
-                  capabilities: [...capabilities, 'vision'],
-                  // Mark this as auto-detected, not user-configured
-                  _autoDetectedVision: true,
-                } as any
 
-                updateProviderState('llamacpp', { models: updatedModels })
+                if (
-                console.log(
+                  !capabilities.includes('vision') &&
-                  `Vision capability added to model after provider refresh: ${importedModelName}`
+                  !hasUserConfiguredCapabilities
-                )
+                ) {
+                  const updatedModels = [...llamacppProvider.models]
+                  updatedModels[modelIndex] = {
+                    ...model,
+                    capabilities: [...capabilities, 'vision'],
+                    // Mark this as auto-detected, not user-configured
+                    _autoDetectedVision: true,
+                  } as any
+
+                  updateProviderState('llamacpp', { models: updatedModels })
+                  console.log(
+                    `Vision capability added to model after provider refresh: ${importedModelName}`
+                  )
+                }
               }
             }
           }
+        } catch (error) {
+          console.error('Error checking mmproj existence after import:', error)
         }
-      } catch (error) {
-        console.error('Error checking mmproj existence after import:', error)
       }
+    } finally {
+      // The importing state will be cleared by the useEffect when model appears in list
     }
   }
 
@@ -175,6 +184,29 @@ function ProviderDetail() {
     return () => clearInterval(intervalId)
   }, [serviceHub, setActiveModels])
 
+  // Clear importing state when model appears in the provider's model list
+  useEffect(() => {
+    if (importingModel && provider?.models) {
+      const modelExists = provider.models.some(
+        (model) => model.id === importingModel
+      )
+      if (modelExists) {
+        setImportingModel(null)
+      }
+    }
+  }, [importingModel, provider?.models])
+
+  // Fallback: Clear importing state after 10 seconds to prevent infinite loading
+  useEffect(() => {
+    if (importingModel) {
+      const timeoutId = setTimeout(() => {
+        setImportingModel(null)
+      }, 10000) // 10 seconds fallback
+
+      return () => clearTimeout(timeoutId)
+    }
+  }, [importingModel])
+
   // Auto-refresh provider settings to get updated backend configuration
   const refreshSettings = useCallback(async () => {
     if (!provider) return
@@ -831,6 +863,28 @@ function ProviderDetail() {
                       </p>
                     </div>
                   )}
+                  {/* Show importing skeleton first if there's one */}
+                  {importingModel && (
+                    <CardItem
+                      key="importing-skeleton"
+                      title={
+                        <div className="flex items-center gap-2">
+                          <div className="flex items-center gap-2 animate-pulse">
+                            <div className="bg-accent/20 flex gap-2 text-accent px-2 py-1 rounded-full text-xs">
+                              <IconLoader
+                                size={16}
+                                className="animate-spin text-accent"
+                              />
+                              Importing...
+                            </div>
+                            <h1 className="font-medium line-clamp-1">
+                              {importingModel}
+                            </h1>
+                          </div>
+                        </div>
+                      }
+                    />
+                  )}
                 </Card>
               </div>
             </div>