Skip to content

Commit 9380774

Browse files
urmauururmauur
authored
Merge pull request #6477 from menloresearch/fix/valdidate-mmproj
fix: validate mmproj from general basename
2 parents fd05214 + 0945eae commit 9380774

File tree

2 files changed

+140
-79
lines changed

2 files changed

+140
-79
lines changed

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

Lines changed: 40 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
import { Button } from '@/components/ui/button'
1111
import { Switch } from '@/components/ui/switch'
1212
import { useServiceHub } from '@/hooks/useServiceHub'
13-
import { useState } from 'react'
13+
import { useState, useEffect, useCallback } from 'react'
1414
import { toast } from 'sonner'
1515
import {
1616
IconLoader2,
@@ -44,7 +44,7 @@ export const ImportVisionModelDialog = ({
4444
>(null)
4545
const [isValidatingMmproj, setIsValidatingMmproj] = useState(false)
4646

47-
const validateGgufFile = async (
47+
const validateGgufFile = useCallback(async (
4848
filePath: string,
4949
fileType: 'model' | 'mmproj'
5050
): Promise<void> => {
@@ -57,25 +57,23 @@ export const ImportVisionModelDialog = ({
5757
}
5858

5959
try {
60-
console.log(`Reading GGUF metadata for ${fileType}:`, filePath)
61-
6260
// Handle validation differently for model files vs mmproj files
6361
if (fileType === 'model') {
6462
// For model files, use the standard validateGgufFile method
6563
if (typeof serviceHub.models().validateGgufFile === 'function') {
6664
const result = await serviceHub.models().validateGgufFile(filePath)
6765

6866
if (result.metadata) {
69-
// Log full metadata for debugging
70-
console.log(
71-
`Full GGUF metadata for ${fileType}:`,
72-
JSON.stringify(result.metadata, null, 2)
73-
)
74-
7567
// Check architecture from metadata
7668
const architecture =
7769
result.metadata.metadata?.['general.architecture']
78-
console.log(`${fileType} architecture:`, architecture)
70+
71+
// Extract baseName and use it as model name if available
72+
const baseName = result.metadata.metadata?.['general.basename']
73+
74+
if (baseName) {
75+
setModelName(baseName)
76+
}
7977

8078
// Model files should NOT be clip
8179
if (architecture === 'clip') {
@@ -86,11 +84,6 @@ export const ImportVisionModelDialog = ({
8684
'CLIP architecture detected in model file:',
8785
architecture
8886
)
89-
} else {
90-
console.log(
91-
'Model validation passed. Architecture:',
92-
architecture
93-
)
9487
}
9588
}
9689

@@ -109,16 +102,15 @@ export const ImportVisionModelDialog = ({
109102
path: filePath,
110103
})
111104

112-
console.log(
113-
`Full GGUF metadata for ${fileType}:`,
114-
JSON.stringify(metadata, null, 2)
115-
)
116105

117106
// Check if architecture matches expected type
118107
const architecture = (
119108
metadata as { metadata?: Record<string, string> }
120109
).metadata?.['general.architecture']
121-
console.log(`${fileType} architecture:`, architecture)
110+
111+
// Get general.baseName from metadata
112+
const baseName = (metadata as { metadata?: Record<string, string> })
113+
.metadata?.['general.basename']
122114

123115
// MMProj files MUST be clip
124116
if (architecture !== 'clip') {
@@ -128,11 +120,19 @@ export const ImportVisionModelDialog = ({
128120
'Non-CLIP architecture detected in mmproj file:',
129121
architecture
130122
)
131-
} else {
132-
console.log(
133-
'MMProj validation passed. Architecture:',
134-
architecture
135-
)
123+
} else if (
124+
baseName &&
125+
modelName &&
126+
!modelName.toLowerCase().includes(baseName.toLowerCase()) &&
127+
!baseName.toLowerCase().includes(modelName.toLowerCase())
128+
) {
129+
// Validate that baseName and model name are compatible (one should contain the other)
130+
const errorMessage = `MMProj file baseName "${baseName}" does not match model name "${modelName}". The MMProj file should be compatible with the selected model.`
131+
setMmprojValidationError(errorMessage)
132+
console.error('BaseName mismatch in mmproj file:', {
133+
baseName,
134+
modelName,
135+
})
136136
}
137137
} catch (directError) {
138138
console.error('Failed to validate mmproj file directly:', directError)
@@ -158,15 +158,15 @@ export const ImportVisionModelDialog = ({
158158
setIsValidatingMmproj(false)
159159
}
160160
}
161-
}
161+
}, [modelName, serviceHub])
162162

163-
const validateModelFile = async (filePath: string): Promise<void> => {
163+
const validateModelFile = useCallback(async (filePath: string): Promise<void> => {
164164
await validateGgufFile(filePath, 'model')
165-
}
165+
}, [validateGgufFile])
166166

167-
const validateMmprojFile = async (filePath: string): Promise<void> => {
167+
const validateMmprojFile = useCallback(async (filePath: string): Promise<void> => {
168168
await validateGgufFile(filePath, 'mmproj')
169-
}
169+
}, [validateGgufFile])
170170

171171
const handleFileSelect = async (type: 'model' | 'mmproj') => {
172172
const selectedFile = await serviceHub.dialog().open({
@@ -179,14 +179,14 @@ export const ImportVisionModelDialog = ({
179179

180180
if (type === 'model') {
181181
setModelFile(selectedFile)
182-
// Auto-generate model name from GGUF file
182+
// Set temporary model name from filename (will be overridden by baseName from metadata if available)
183183
const sanitizedName = fileName
184184
.replace(/\s/g, '-')
185185
.replace(/\.(gguf|GGUF)$/, '')
186186
.replace(/[^a-zA-Z0-9/_.-]/g, '') // Remove any characters not allowed in model IDs
187187
setModelName(sanitizedName)
188188

189-
// Validate the selected model file
189+
// Validate the selected model file (this will update model name with baseName from metadata)
190190
await validateModelFile(selectedFile)
191191
} else {
192192
setMmProjFile(selectedFile)
@@ -272,6 +272,13 @@ export const ImportVisionModelDialog = ({
272272
setIsValidatingMmproj(false)
273273
}
274274

275+
// Re-validate MMProj file when model name changes
276+
useEffect(() => {
277+
if (mmProjFile && modelName && isVisionModel) {
278+
validateMmprojFile(mmProjFile)
279+
}
280+
}, [modelName, mmProjFile, isVisionModel, validateMmprojFile])
281+
275282
const handleOpenChange = (newOpen: boolean) => {
276283
if (!importing) {
277284
setOpen(newOpen)

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

Lines changed: 100 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ function ProviderDetail() {
8383
const [refreshingModels, setRefreshingModels] = useState(false)
8484
const [isCheckingBackendUpdate, setIsCheckingBackendUpdate] = useState(false)
8585
const [isInstallingBackend, setIsInstallingBackend] = useState(false)
86+
const [importingModel, setImportingModel] = useState<string | null>(null)
8687
const { checkForUpdate: checkForBackendUpdate, installBackend } =
8788
useBackendUpdater()
8889
const { providerName } = useParams({ from: Route.id })
@@ -102,58 +103,66 @@ function ProviderDetail() {
102103
)
103104

104105
const handleModelImportSuccess = async (importedModelName?: string) => {
105-
// Refresh the provider to update the models list
106-
await serviceHub.providers().getProviders().then(setProviders)
106+
if (importedModelName) {
107+
setImportingModel(importedModelName)
108+
}
107109

108-
// If a model was imported and it might have vision capabilities, check and update
109-
if (importedModelName && providerName === 'llamacpp') {
110-
try {
111-
const mmprojExists = await serviceHub
112-
.models()
113-
.checkMmprojExists(importedModelName)
114-
if (mmprojExists) {
115-
// Get the updated provider after refresh
116-
const { getProviderByName, updateProvider: updateProviderState } =
117-
useModelProvider.getState()
118-
const llamacppProvider = getProviderByName('llamacpp')
119-
120-
if (llamacppProvider) {
121-
const modelIndex = llamacppProvider.models.findIndex(
122-
(m: Model) => m.id === importedModelName
123-
)
124-
if (modelIndex !== -1) {
125-
const model = llamacppProvider.models[modelIndex]
126-
const capabilities = model.capabilities || []
127-
128-
// Add 'vision' capability if not already present AND if user hasn't manually configured capabilities
129-
// Check if model has a custom capabilities config flag
130-
131-
const hasUserConfiguredCapabilities =
132-
(model as any)._userConfiguredCapabilities === true
133-
134-
if (
135-
!capabilities.includes('vision') &&
136-
!hasUserConfiguredCapabilities
137-
) {
138-
const updatedModels = [...llamacppProvider.models]
139-
updatedModels[modelIndex] = {
140-
...model,
141-
capabilities: [...capabilities, 'vision'],
142-
// Mark this as auto-detected, not user-configured
143-
_autoDetectedVision: true,
144-
} as any
145-
146-
updateProviderState('llamacpp', { models: updatedModels })
147-
console.log(
148-
`Vision capability added to model after provider refresh: ${importedModelName}`
149-
)
110+
try {
111+
// Refresh the provider to update the models list
112+
await serviceHub.providers().getProviders().then(setProviders)
113+
114+
// If a model was imported and it might have vision capabilities, check and update
115+
if (importedModelName && providerName === 'llamacpp') {
116+
try {
117+
const mmprojExists = await serviceHub
118+
.models()
119+
.checkMmprojExists(importedModelName)
120+
if (mmprojExists) {
121+
// Get the updated provider after refresh
122+
const { getProviderByName, updateProvider: updateProviderState } =
123+
useModelProvider.getState()
124+
const llamacppProvider = getProviderByName('llamacpp')
125+
126+
if (llamacppProvider) {
127+
const modelIndex = llamacppProvider.models.findIndex(
128+
(m: Model) => m.id === importedModelName
129+
)
130+
if (modelIndex !== -1) {
131+
const model = llamacppProvider.models[modelIndex]
132+
const capabilities = model.capabilities || []
133+
134+
// Add 'vision' capability if not already present AND if user hasn't manually configured capabilities
135+
// Check if model has a custom capabilities config flag
136+
137+
const hasUserConfiguredCapabilities =
138+
(model as any)._userConfiguredCapabilities === true
139+
140+
if (
141+
!capabilities.includes('vision') &&
142+
!hasUserConfiguredCapabilities
143+
) {
144+
const updatedModels = [...llamacppProvider.models]
145+
updatedModels[modelIndex] = {
146+
...model,
147+
capabilities: [...capabilities, 'vision'],
148+
// Mark this as auto-detected, not user-configured
149+
_autoDetectedVision: true,
150+
} as any
151+
152+
updateProviderState('llamacpp', { models: updatedModels })
153+
console.log(
154+
`Vision capability added to model after provider refresh: ${importedModelName}`
155+
)
156+
}
150157
}
151158
}
152159
}
160+
} catch (error) {
161+
console.error('Error checking mmproj existence after import:', error)
153162
}
154-
} catch (error) {
155-
console.error('Error checking mmproj existence after import:', error)
156163
}
164+
} finally {
165+
// The importing state will be cleared by the useEffect when model appears in list
157166
}
158167
}
159168

@@ -175,6 +184,29 @@ function ProviderDetail() {
175184
return () => clearInterval(intervalId)
176185
}, [serviceHub, setActiveModels])
177186

187+
// Clear importing state when model appears in the provider's model list
188+
useEffect(() => {
189+
if (importingModel && provider?.models) {
190+
const modelExists = provider.models.some(
191+
(model) => model.id === importingModel
192+
)
193+
if (modelExists) {
194+
setImportingModel(null)
195+
}
196+
}
197+
}, [importingModel, provider?.models])
198+
199+
// Fallback: Clear importing state after 10 seconds to prevent infinite loading
200+
useEffect(() => {
201+
if (importingModel) {
202+
const timeoutId = setTimeout(() => {
203+
setImportingModel(null)
204+
}, 10000) // 10 seconds fallback
205+
206+
return () => clearTimeout(timeoutId)
207+
}
208+
}, [importingModel])
209+
178210
// Auto-refresh provider settings to get updated backend configuration
179211
const refreshSettings = useCallback(async () => {
180212
if (!provider) return
@@ -831,6 +863,28 @@ function ProviderDetail() {
831863
</p>
832864
</div>
833865
)}
866+
{/* Show importing skeleton first if there's one */}
867+
{importingModel && (
868+
<CardItem
869+
key="importing-skeleton"
870+
title={
871+
<div className="flex items-center gap-2">
872+
<div className="flex items-center gap-2 animate-pulse">
873+
<div className="bg-accent/20 flex gap-2 text-accent px-2 py-1 rounded-full text-xs">
874+
<IconLoader
875+
size={16}
876+
className="animate-spin text-accent"
877+
/>
878+
Importing...
879+
</div>
880+
<h1 className="font-medium line-clamp-1">
881+
{importingModel}
882+
</h1>
883+
</div>
884+
</div>
885+
}
886+
/>
887+
)}
834888
</Card>
835889
</div>
836890
</div>

0 commit comments

Comments
 (0)