Merge pull request #6643 from menloresearch/fix/model-name-change
fix: Apply model name change correctly
This commit is contained in:
commit
70ac13e536
1
src-tauri/Cargo.lock
generated
1
src-tauri/Cargo.lock
generated
@ -5323,6 +5323,7 @@ dependencies = [
|
||||
"sysinfo",
|
||||
"tauri",
|
||||
"tauri-plugin",
|
||||
"tauri-plugin-hardware",
|
||||
"thiserror 2.0.12",
|
||||
"tokio",
|
||||
]
|
||||
|
||||
@ -6,7 +6,7 @@ import {
|
||||
PopoverTrigger,
|
||||
} from '@/components/ui/popover'
|
||||
import { useModelProvider } from '@/hooks/useModelProvider'
|
||||
import { cn, getProviderTitle } from '@/lib/utils'
|
||||
import { cn, getProviderTitle, getModelDisplayName } from '@/lib/utils'
|
||||
import { highlightFzfMatch } from '@/utils/highlight'
|
||||
import Capabilities from './Capabilities'
|
||||
import { IconSettings, IconX } from '@tabler/icons-react'
|
||||
@ -240,7 +240,7 @@ const DropdownModelProvider = ({
|
||||
// Update display model when selection changes
|
||||
useEffect(() => {
|
||||
if (selectedProvider && selectedModel) {
|
||||
setDisplayModel(selectedModel.id)
|
||||
setDisplayModel(getModelDisplayName(selectedModel))
|
||||
} else {
|
||||
setDisplayModel(t('common:selectAModel'))
|
||||
}
|
||||
@ -326,7 +326,7 @@ const DropdownModelProvider = ({
|
||||
// Create Fzf instance for fuzzy search
|
||||
const fzfInstance = useMemo(() => {
|
||||
return new Fzf(searchableItems, {
|
||||
selector: (item) => item.model.id.toLowerCase(),
|
||||
selector: (item) => `${getModelDisplayName(item.model)} ${item.model.id}`.toLowerCase(),
|
||||
})
|
||||
}, [searchableItems])
|
||||
|
||||
@ -390,7 +390,7 @@ const DropdownModelProvider = ({
|
||||
const handleSelect = useCallback(
|
||||
async (searchableModel: SearchableModel) => {
|
||||
// Immediately update display to prevent double-click issues
|
||||
setDisplayModel(searchableModel.model.id)
|
||||
setDisplayModel(getModelDisplayName(searchableModel.model))
|
||||
setSearchValue('')
|
||||
setOpen(false)
|
||||
|
||||
@ -576,7 +576,7 @@ const DropdownModelProvider = ({
|
||||
/>
|
||||
</div>
|
||||
<span className="text-main-view-fg/80 text-sm">
|
||||
{searchableModel.model.id}
|
||||
{getModelDisplayName(searchableModel.model)}
|
||||
</span>
|
||||
<div className="flex-1"></div>
|
||||
{capabilities.length > 0 && (
|
||||
@ -669,7 +669,7 @@ const DropdownModelProvider = ({
|
||||
className="text-main-view-fg/80 text-sm"
|
||||
title={searchableModel.model.id}
|
||||
>
|
||||
{searchableModel.model.id}
|
||||
{getModelDisplayName(searchableModel.model)}
|
||||
</span>
|
||||
<div className="flex-1"></div>
|
||||
{capabilities.length > 0 && (
|
||||
|
||||
@ -14,7 +14,7 @@ import { Button } from '@/components/ui/button'
|
||||
import { DynamicControllerSetting } from '@/containers/dynamicControllerSetting'
|
||||
import { useModelProvider } from '@/hooks/useModelProvider'
|
||||
import { useServiceHub } from '@/hooks/useServiceHub'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { cn, getModelDisplayName } from '@/lib/utils'
|
||||
import { useTranslation } from '@/i18n/react-i18next-compat'
|
||||
|
||||
type ModelSettingProps = {
|
||||
@ -261,7 +261,7 @@ export function ModelSetting({
|
||||
<SheetContent className="h-[calc(100%-8px)] top-1 right-1 rounded-e-md overflow-y-auto">
|
||||
<SheetHeader>
|
||||
<SheetTitle>
|
||||
{t('common:modelSettings.title', { modelId: model.id })}
|
||||
{t('common:modelSettings.title', { modelId: getModelDisplayName(model) })}
|
||||
</SheetTitle>
|
||||
<SheetDescription>
|
||||
{t('common:modelSettings.description')}
|
||||
|
||||
@ -0,0 +1,277 @@
|
||||
import { describe, it, expect, beforeEach, vi } from 'vitest'
|
||||
import { render, screen } from '@testing-library/react'
|
||||
import '@testing-library/jest-dom'
|
||||
import DropdownModelProvider from '../DropdownModelProvider'
|
||||
import { getModelDisplayName } from '@/lib/utils'
|
||||
import { useModelProvider } from '@/hooks/useModelProvider'
|
||||
|
||||
// Define basic types to avoid missing declarations
|
||||
type ModelProvider = {
|
||||
provider: string
|
||||
active: boolean
|
||||
models: Array<{
|
||||
id: string
|
||||
displayName?: string
|
||||
capabilities: string[]
|
||||
}>
|
||||
settings: unknown[]
|
||||
}
|
||||
|
||||
type Model = {
|
||||
id: string
|
||||
displayName?: string
|
||||
capabilities?: string[]
|
||||
}
|
||||
|
||||
type MockHookReturn = {
|
||||
providers: ModelProvider[]
|
||||
selectedProvider: string
|
||||
selectedModel: Model
|
||||
getProviderByName: (name: string) => ModelProvider | undefined
|
||||
selectModelProvider: () => void
|
||||
getModelBy: (id: string) => Model | undefined
|
||||
updateProvider: () => void
|
||||
}
|
||||
|
||||
// Mock the dependencies
|
||||
vi.mock('@/hooks/useModelProvider', () => ({
|
||||
useModelProvider: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('@/hooks/useThreads', () => ({
|
||||
useThreads: vi.fn(() => ({
|
||||
updateCurrentThreadModel: vi.fn(),
|
||||
})),
|
||||
}))
|
||||
|
||||
vi.mock('@/hooks/useServiceHub', () => ({
|
||||
useServiceHub: vi.fn(() => ({
|
||||
models: () => ({
|
||||
checkMmprojExists: vi.fn(() => Promise.resolve(false)),
|
||||
checkMmprojExistsAndUpdateOffloadMMprojSetting: vi.fn(() => Promise.resolve()),
|
||||
}),
|
||||
})),
|
||||
}))
|
||||
|
||||
vi.mock('@/i18n/react-i18next-compat', () => ({
|
||||
useTranslation: vi.fn(() => ({
|
||||
t: (key: string) => key,
|
||||
})),
|
||||
}))
|
||||
|
||||
vi.mock('@tanstack/react-router', () => ({
|
||||
useNavigate: vi.fn(() => vi.fn()),
|
||||
}))
|
||||
|
||||
vi.mock('@/hooks/useFavoriteModel', () => ({
|
||||
useFavoriteModel: vi.fn(() => ({
|
||||
favoriteModels: [],
|
||||
})),
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/platform/const', () => ({
|
||||
PlatformFeatures: {
|
||||
WEB_AUTO_MODEL_SELECTION: false,
|
||||
MODEL_PROVIDER_SETTINGS: true,
|
||||
},
|
||||
}))
|
||||
|
||||
// Mock UI components
|
||||
vi.mock('@/components/ui/popover', () => ({
|
||||
Popover: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||
PopoverTrigger: ({ children }: { children: React.ReactNode }) => (
|
||||
<div data-testid="popover-trigger">{children}</div>
|
||||
),
|
||||
PopoverContent: ({ children }: { children: React.ReactNode }) => (
|
||||
<div data-testid="popover-content">{children}</div>
|
||||
),
|
||||
}))
|
||||
|
||||
vi.mock('../ProvidersAvatar', () => ({
|
||||
default: ({ provider }: { provider: any }) => (
|
||||
<div data-testid={`provider-avatar-${provider.provider}`} />
|
||||
),
|
||||
}))
|
||||
|
||||
vi.mock('../Capabilities', () => ({
|
||||
default: ({ capabilities }: { capabilities: string[] }) => (
|
||||
<div data-testid="capabilities">{capabilities.join(',')}</div>
|
||||
),
|
||||
}))
|
||||
|
||||
vi.mock('../ModelSetting', () => ({
|
||||
ModelSetting: () => <div data-testid="model-setting" />,
|
||||
}))
|
||||
|
||||
vi.mock('../ModelSupportStatus', () => ({
|
||||
ModelSupportStatus: () => <div data-testid="model-support-status" />,
|
||||
}))
|
||||
|
||||
describe('DropdownModelProvider - Display Name Integration', () => {
|
||||
const mockProviders: ModelProvider[] = [
|
||||
{
|
||||
provider: 'llamacpp',
|
||||
active: true,
|
||||
models: [
|
||||
{
|
||||
id: 'model1.gguf',
|
||||
displayName: 'Custom Model 1',
|
||||
capabilities: ['completion'],
|
||||
},
|
||||
{
|
||||
id: 'model2-very-long-filename.gguf',
|
||||
displayName: 'Short Name',
|
||||
capabilities: ['completion'],
|
||||
},
|
||||
{
|
||||
id: 'model3.gguf',
|
||||
// No displayName - should fall back to ID
|
||||
capabilities: ['completion'],
|
||||
},
|
||||
],
|
||||
settings: [],
|
||||
},
|
||||
]
|
||||
|
||||
const mockSelectedModel = {
|
||||
id: 'model1.gguf',
|
||||
displayName: 'Custom Model 1',
|
||||
capabilities: ['completion'],
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
|
||||
// Reset the mock for each test
|
||||
vi.mocked(useModelProvider).mockReturnValue({
|
||||
providers: mockProviders,
|
||||
selectedProvider: 'llamacpp',
|
||||
selectedModel: mockSelectedModel,
|
||||
getProviderByName: vi.fn((name: string) =>
|
||||
mockProviders.find((p: ModelProvider) => p.provider === name)
|
||||
),
|
||||
selectModelProvider: vi.fn(),
|
||||
getModelBy: vi.fn((id: string) =>
|
||||
mockProviders[0].models.find((m: Model) => m.id === id)
|
||||
),
|
||||
updateProvider: vi.fn(),
|
||||
} as MockHookReturn)
|
||||
})
|
||||
|
||||
it('should display custom model name in the trigger button', () => {
|
||||
render(<DropdownModelProvider />)
|
||||
|
||||
// Should show the display name in both trigger and dropdown
|
||||
expect(screen.getAllByText('Custom Model 1')).toHaveLength(2) // One in trigger, one in dropdown
|
||||
// Model ID should not be visible as text (it's only in title attributes)
|
||||
expect(screen.queryByDisplayValue('model1.gguf')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('should fall back to model ID when no displayName is set', () => {
|
||||
vi.mocked(useModelProvider).mockReturnValue({
|
||||
providers: mockProviders,
|
||||
selectedProvider: 'llamacpp',
|
||||
selectedModel: mockProviders[0].models[2], // model3 without displayName
|
||||
getProviderByName: vi.fn((name: string) =>
|
||||
mockProviders.find((p: ModelProvider) => p.provider === name)
|
||||
),
|
||||
selectModelProvider: vi.fn(),
|
||||
getModelBy: vi.fn((id: string) =>
|
||||
mockProviders[0].models.find((m: Model) => m.id === id)
|
||||
),
|
||||
updateProvider: vi.fn(),
|
||||
} as MockHookReturn)
|
||||
|
||||
render(<DropdownModelProvider />)
|
||||
|
||||
expect(screen.getAllByText('model3.gguf')).toHaveLength(2) // Trigger and dropdown
|
||||
})
|
||||
|
||||
it('should show display names in the model list items', () => {
|
||||
render(<DropdownModelProvider />)
|
||||
|
||||
// Check if the display names are shown in the options
|
||||
expect(screen.getAllByText('Custom Model 1')).toHaveLength(2) // Selected: Trigger + dropdown
|
||||
expect(screen.getByText('Short Name')).toBeInTheDocument() // Only in dropdown
|
||||
expect(screen.getByText('model3.gguf')).toBeInTheDocument() // Only in dropdown
|
||||
})
|
||||
|
||||
it('should use getModelDisplayName utility correctly', () => {
|
||||
// Test the utility function directly with different model scenarios
|
||||
const modelWithDisplayName = {
|
||||
id: 'long-model-name.gguf',
|
||||
displayName: 'Short Name',
|
||||
} as Model
|
||||
|
||||
const modelWithoutDisplayName = {
|
||||
id: 'model-without-display-name.gguf',
|
||||
} as Model
|
||||
|
||||
const modelWithEmptyDisplayName = {
|
||||
id: 'model-with-empty.gguf',
|
||||
displayName: '',
|
||||
} as Model
|
||||
|
||||
expect(getModelDisplayName(modelWithDisplayName)).toBe('Short Name')
|
||||
expect(getModelDisplayName(modelWithoutDisplayName)).toBe('model-without-display-name.gguf')
|
||||
expect(getModelDisplayName(modelWithEmptyDisplayName)).toBe('model-with-empty.gguf')
|
||||
})
|
||||
|
||||
it('should maintain model ID for internal operations while showing display name', () => {
|
||||
const mockSelectModelProvider = vi.fn()
|
||||
|
||||
vi.mocked(useModelProvider).mockReturnValue({
|
||||
providers: mockProviders,
|
||||
selectedProvider: 'llamacpp',
|
||||
selectedModel: mockSelectedModel,
|
||||
getProviderByName: vi.fn((name: string) =>
|
||||
mockProviders.find((p: ModelProvider) => p.provider === name)
|
||||
),
|
||||
selectModelProvider: mockSelectModelProvider,
|
||||
getModelBy: vi.fn((id: string) =>
|
||||
mockProviders[0].models.find((m: Model) => m.id === id)
|
||||
),
|
||||
updateProvider: vi.fn(),
|
||||
} as MockHookReturn)
|
||||
|
||||
render(<DropdownModelProvider />)
|
||||
|
||||
// Verify that display name is shown in UI
|
||||
expect(screen.getAllByText('Custom Model 1')).toHaveLength(2) // Trigger + dropdown
|
||||
|
||||
// The actual model ID should still be preserved for backend operations
|
||||
// This would be tested in the click handlers, but that requires more complex mocking
|
||||
expect(mockSelectedModel.id).toBe('model1.gguf')
|
||||
})
|
||||
|
||||
it('should handle updating display model when selection changes', () => {
|
||||
// Test that when a new model is selected, the trigger updates correctly
|
||||
// First render with model1 selected
|
||||
const { rerender } = render(<DropdownModelProvider />)
|
||||
|
||||
// Check trigger shows Custom Model 1
|
||||
const triggerButton = screen.getByRole('button')
|
||||
expect(triggerButton).toHaveTextContent('Custom Model 1')
|
||||
|
||||
// Update to select model2
|
||||
vi.mocked(useModelProvider).mockReturnValue({
|
||||
providers: mockProviders,
|
||||
selectedProvider: 'llamacpp',
|
||||
selectedModel: mockProviders[0].models[1], // model2
|
||||
getProviderByName: vi.fn((name: string) =>
|
||||
mockProviders.find((p: ModelProvider) => p.provider === name)
|
||||
),
|
||||
selectModelProvider: vi.fn(),
|
||||
getModelBy: vi.fn((id: string) =>
|
||||
mockProviders[0].models.find((m: Model) => m.id === id)
|
||||
),
|
||||
updateProvider: vi.fn(),
|
||||
} as MockHookReturn)
|
||||
|
||||
rerender(<DropdownModelProvider />)
|
||||
// Check trigger now shows Short Name
|
||||
expect(triggerButton).toHaveTextContent('Short Name')
|
||||
// Both models are still visible in the dropdown, so we can't test for absence
|
||||
expect(screen.getAllByText('Short Name')).toHaveLength(2) // trigger + dropdown
|
||||
})
|
||||
})
|
||||
184
web-app/src/containers/__tests__/EditModel.test.tsx
Normal file
184
web-app/src/containers/__tests__/EditModel.test.tsx
Normal file
@ -0,0 +1,184 @@
|
||||
import { describe, it, expect, beforeEach, vi } from 'vitest'
|
||||
import { render, screen, fireEvent, waitFor } from '@testing-library/react'
|
||||
import userEvent from '@testing-library/user-event'
|
||||
import { DialogEditModel } from '../dialogs/EditModel'
|
||||
import { useModelProvider } from '@/hooks/useModelProvider'
|
||||
import '@testing-library/jest-dom'
|
||||
|
||||
// Mock the dependencies
|
||||
vi.mock('@/hooks/useModelProvider', () => ({
|
||||
useModelProvider: vi.fn(() => ({
|
||||
updateProvider: vi.fn(),
|
||||
setProviders: vi.fn(),
|
||||
})),
|
||||
}))
|
||||
|
||||
vi.mock('@/hooks/useServiceHub', () => ({
|
||||
useServiceHub: vi.fn(() => ({
|
||||
providers: () => ({
|
||||
getProviders: vi.fn(() => Promise.resolve([])),
|
||||
}),
|
||||
})),
|
||||
}))
|
||||
|
||||
vi.mock('@/i18n/react-i18next-compat', () => ({
|
||||
useTranslation: vi.fn(() => ({
|
||||
t: (key: string) => key,
|
||||
})),
|
||||
}))
|
||||
|
||||
vi.mock('sonner', () => ({
|
||||
toast: {
|
||||
success: vi.fn(),
|
||||
error: vi.fn(),
|
||||
},
|
||||
}))
|
||||
|
||||
// Mock Dialog components
|
||||
vi.mock('@/components/ui/dialog', () => ({
|
||||
Dialog: ({ children, open }: { children: React.ReactNode; open: boolean }) =>
|
||||
open ? <div data-testid="dialog">{children}</div> : null,
|
||||
DialogContent: ({ children }: { children: React.ReactNode }) => (
|
||||
<div data-testid="dialog-content">{children}</div>
|
||||
),
|
||||
DialogHeader: ({ children }: { children: React.ReactNode }) => (
|
||||
<div data-testid="dialog-header">{children}</div>
|
||||
),
|
||||
DialogTitle: ({ children }: { children: React.ReactNode }) => (
|
||||
<h1 data-testid="dialog-title">{children}</h1>
|
||||
),
|
||||
DialogDescription: ({ children }: { children: React.ReactNode }) => (
|
||||
<p data-testid="dialog-description">{children}</p>
|
||||
),
|
||||
DialogTrigger: ({ children }: { children: React.ReactNode }) => (
|
||||
<div data-testid="dialog-trigger">{children}</div>
|
||||
),
|
||||
}))
|
||||
|
||||
vi.mock('@/components/ui/input', () => ({
|
||||
Input: ({ value, onChange, ...props }: any) => (
|
||||
<input
|
||||
value={value}
|
||||
onChange={onChange}
|
||||
data-testid="display-name-input"
|
||||
{...props}
|
||||
/>
|
||||
),
|
||||
}))
|
||||
|
||||
vi.mock('@/components/ui/button', () => ({
|
||||
Button: ({ children, onClick, ...props }: any) => (
|
||||
<button onClick={onClick} data-testid="button" {...props}>
|
||||
{children}
|
||||
</button>
|
||||
),
|
||||
}))
|
||||
|
||||
// Mock other UI components
|
||||
vi.mock('@tabler/icons-react', () => ({
|
||||
IconPencil: () => <div data-testid="pencil-icon" />,
|
||||
IconCheck: () => <div data-testid="check-icon" />,
|
||||
IconX: () => <div data-testid="x-icon" />,
|
||||
IconAlertTriangle: () => <div data-testid="alert-triangle-icon" />,
|
||||
IconEye: () => <div data-testid="eye-icon" />,
|
||||
IconTool: () => <div data-testid="tool-icon" />,
|
||||
IconLoader2: () => <div data-testid="loader-icon" />,
|
||||
}))
|
||||
|
||||
describe('DialogEditModel - Basic Component Tests', () => {
|
||||
const mockProvider = {
|
||||
provider: 'llamacpp',
|
||||
active: true,
|
||||
models: [
|
||||
{
|
||||
id: 'test-model.gguf',
|
||||
displayName: 'My Custom Model',
|
||||
capabilities: ['completion'],
|
||||
},
|
||||
],
|
||||
settings: [],
|
||||
} as any
|
||||
|
||||
const mockUpdateProvider = vi.fn()
|
||||
const mockSetProviders = vi.fn()
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
|
||||
vi.mocked(useModelProvider).mockReturnValue({
|
||||
updateProvider: mockUpdateProvider,
|
||||
setProviders: mockSetProviders,
|
||||
} as any)
|
||||
})
|
||||
|
||||
it('should render without errors', () => {
|
||||
const { container } = render(
|
||||
<DialogEditModel
|
||||
provider={mockProvider}
|
||||
modelId="test-model.gguf"
|
||||
/>
|
||||
)
|
||||
|
||||
// Component should render without throwing errors
|
||||
expect(container).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('should handle provider without models', () => {
|
||||
const emptyProvider = {
|
||||
...mockProvider,
|
||||
models: [],
|
||||
} as any
|
||||
|
||||
const { container } = render(
|
||||
<DialogEditModel
|
||||
provider={emptyProvider}
|
||||
modelId="test-model.gguf"
|
||||
/>
|
||||
)
|
||||
|
||||
// Component should handle empty models gracefully
|
||||
expect(container).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('should accept provider and modelId props', () => {
|
||||
const { container } = render(
|
||||
<DialogEditModel
|
||||
provider={mockProvider}
|
||||
modelId="different-model.gguf"
|
||||
/>
|
||||
)
|
||||
|
||||
expect(container).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('should not crash with minimal props', () => {
|
||||
const minimalProvider = {
|
||||
provider: 'test',
|
||||
active: false,
|
||||
models: [],
|
||||
settings: [],
|
||||
} as any
|
||||
|
||||
expect(() => {
|
||||
render(
|
||||
<DialogEditModel
|
||||
provider={minimalProvider}
|
||||
modelId="any-model"
|
||||
/>
|
||||
)
|
||||
}).not.toThrow()
|
||||
})
|
||||
|
||||
it('should have mocked dependencies available', () => {
|
||||
render(
|
||||
<DialogEditModel
|
||||
provider={mockProvider}
|
||||
modelId="test-model.gguf"
|
||||
/>
|
||||
)
|
||||
|
||||
// Verify our mocks are in place
|
||||
expect(mockUpdateProvider).toBeDefined()
|
||||
expect(mockSetProviders).toBeDefined()
|
||||
})
|
||||
})
|
||||
@ -23,7 +23,6 @@ import {
|
||||
} from '@tabler/icons-react'
|
||||
import { useState, useEffect } from 'react'
|
||||
import { useTranslation } from '@/i18n/react-i18next-compat'
|
||||
import { useServiceHub } from '@/hooks/useServiceHub'
|
||||
import { toast } from 'sonner'
|
||||
|
||||
// No need to define our own interface, we'll use the existing Model type
|
||||
@ -37,16 +36,15 @@ export const DialogEditModel = ({
|
||||
modelId,
|
||||
}: DialogEditModelProps) => {
|
||||
const { t } = useTranslation()
|
||||
const { updateProvider, setProviders } = useModelProvider()
|
||||
const { updateProvider } = useModelProvider()
|
||||
const [selectedModelId, setSelectedModelId] = useState<string>('')
|
||||
const [modelName, setModelName] = useState<string>('')
|
||||
const [originalModelName, setOriginalModelName] = useState<string>('')
|
||||
const [displayName, setDisplayName] = useState<string>('')
|
||||
const [originalDisplayName, setOriginalDisplayName] = useState<string>('')
|
||||
const [originalCapabilities, setOriginalCapabilities] = useState<
|
||||
Record<string, boolean>
|
||||
>({})
|
||||
const [isOpen, setIsOpen] = useState(false)
|
||||
const [isLoading, setIsLoading] = useState(false)
|
||||
const serviceHub = useServiceHub()
|
||||
const [capabilities, setCapabilities] = useState<Record<string, boolean>>({
|
||||
completion: false,
|
||||
vision: false,
|
||||
@ -82,11 +80,10 @@ export const DialogEditModel = ({
|
||||
|
||||
// Get the currently selected model
|
||||
const selectedModel = provider.models.find(
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
(m: any) => m.id === selectedModelId
|
||||
(m: Model) => m.id === selectedModelId
|
||||
)
|
||||
|
||||
// Initialize capabilities and model name from selected model
|
||||
// Initialize capabilities and display name from selected model
|
||||
useEffect(() => {
|
||||
if (selectedModel) {
|
||||
const modelCapabilities = selectedModel.capabilities || []
|
||||
@ -98,9 +95,10 @@ export const DialogEditModel = ({
|
||||
web_search: modelCapabilities.includes('web_search'),
|
||||
reasoning: modelCapabilities.includes('reasoning'),
|
||||
})
|
||||
const modelNameValue = selectedModel.id
|
||||
setModelName(modelNameValue)
|
||||
setOriginalModelName(modelNameValue)
|
||||
// Use existing displayName if available, otherwise fall back to model ID
|
||||
const displayNameValue = (selectedModel as Model & { displayName?: string }).displayName || selectedModel.id
|
||||
setDisplayName(displayNameValue)
|
||||
setOriginalDisplayName(displayNameValue)
|
||||
|
||||
const originalCaps = {
|
||||
completion: modelCapabilities.includes('completion'),
|
||||
@ -122,14 +120,14 @@ export const DialogEditModel = ({
|
||||
}))
|
||||
}
|
||||
|
||||
// Handle model name change
|
||||
const handleModelNameChange = (newName: string) => {
|
||||
setModelName(newName)
|
||||
// Handle display name change
|
||||
const handleDisplayNameChange = (newName: string) => {
|
||||
setDisplayName(newName)
|
||||
}
|
||||
|
||||
// Check if there are unsaved changes
|
||||
const hasUnsavedChanges = () => {
|
||||
const nameChanged = modelName !== originalModelName
|
||||
const nameChanged = displayName !== originalDisplayName
|
||||
const capabilitiesChanged =
|
||||
JSON.stringify(capabilities) !== JSON.stringify(originalCapabilities)
|
||||
return nameChanged || capabilitiesChanged
|
||||
@ -141,13 +139,21 @@ export const DialogEditModel = ({
|
||||
|
||||
setIsLoading(true)
|
||||
try {
|
||||
// Update model name if changed
|
||||
if (modelName !== originalModelName) {
|
||||
await serviceHub
|
||||
.models()
|
||||
.updateModel(selectedModel.id, { id: modelName })
|
||||
setOriginalModelName(modelName)
|
||||
await serviceHub.providers().getProviders().then(setProviders)
|
||||
let updatedModels = provider.models
|
||||
|
||||
// Update display name if changed
|
||||
if (displayName !== originalDisplayName) {
|
||||
// Update the model in the provider models array with displayName
|
||||
updatedModels = updatedModels.map((m: Model) => {
|
||||
if (m.id === selectedModelId) {
|
||||
return {
|
||||
...m,
|
||||
displayName: displayName,
|
||||
}
|
||||
}
|
||||
return m
|
||||
})
|
||||
setOriginalDisplayName(displayName)
|
||||
}
|
||||
|
||||
// Update capabilities if changed
|
||||
@ -159,8 +165,7 @@ export const DialogEditModel = ({
|
||||
.map(([capName]) => capName)
|
||||
|
||||
// Find and update the model in the provider
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
const updatedModels = provider.models.map((m: any) => {
|
||||
updatedModels = updatedModels.map((m: Model) => {
|
||||
if (m.id === selectedModelId) {
|
||||
return {
|
||||
...m,
|
||||
@ -172,15 +177,15 @@ export const DialogEditModel = ({
|
||||
return m
|
||||
})
|
||||
|
||||
setOriginalCapabilities(capabilities)
|
||||
}
|
||||
|
||||
// Update the provider with the updated models
|
||||
updateProvider(provider.provider, {
|
||||
...provider,
|
||||
models: updatedModels,
|
||||
})
|
||||
|
||||
setOriginalCapabilities(capabilities)
|
||||
}
|
||||
|
||||
// Show success toast and close dialog
|
||||
toast.success('Model updated successfully')
|
||||
setIsOpen(false)
|
||||
@ -213,22 +218,25 @@ export const DialogEditModel = ({
|
||||
</DialogDescription>
|
||||
</DialogHeader>
|
||||
|
||||
{/* Model Name Section */}
|
||||
{/* Model Display Name Section */}
|
||||
<div className="py-1">
|
||||
<label
|
||||
htmlFor="model-name"
|
||||
htmlFor="display-name"
|
||||
className="text-sm font-medium mb-3 block"
|
||||
>
|
||||
Model Name
|
||||
Display Name
|
||||
</label>
|
||||
<Input
|
||||
id="model-name"
|
||||
value={modelName}
|
||||
onChange={(e) => handleModelNameChange(e.target.value)}
|
||||
placeholder="Enter model name"
|
||||
id="display-name"
|
||||
value={displayName}
|
||||
onChange={(e) => handleDisplayNameChange(e.target.value)}
|
||||
placeholder="Enter display name"
|
||||
className="w-full"
|
||||
disabled={isLoading}
|
||||
/>
|
||||
<p className="text-xs text-main-view-fg/60 mt-1">
|
||||
This is the name that will be shown in the interface. The original model file remains unchanged.
|
||||
</p>
|
||||
</div>
|
||||
|
||||
{/* Warning Banner */}
|
||||
|
||||
182
web-app/src/hooks/__tests__/useModelProvider.test.ts
Normal file
182
web-app/src/hooks/__tests__/useModelProvider.test.ts
Normal file
@ -0,0 +1,182 @@
|
||||
import { describe, it, expect, beforeEach, vi } from 'vitest'
|
||||
import { act, renderHook } from '@testing-library/react'
|
||||
import { useModelProvider } from '../useModelProvider'
|
||||
|
||||
// Mock getServiceHub
|
||||
vi.mock('@/hooks/useServiceHub', () => ({
|
||||
getServiceHub: vi.fn(() => ({
|
||||
path: () => ({
|
||||
sep: () => '/',
|
||||
}),
|
||||
})),
|
||||
}))
|
||||
|
||||
// Mock the localStorage key constants
|
||||
vi.mock('@/constants/localStorage', () => ({
|
||||
localStorageKey: {
|
||||
modelProvider: 'jan-model-provider',
|
||||
},
|
||||
}))
|
||||
|
||||
// Mock localStorage
|
||||
const localStorageMock = {
|
||||
getItem: vi.fn(),
|
||||
setItem: vi.fn(),
|
||||
removeItem: vi.fn(),
|
||||
clear: vi.fn(),
|
||||
}
|
||||
Object.defineProperty(window, 'localStorage', {
|
||||
value: localStorageMock,
|
||||
writable: true,
|
||||
})
|
||||
|
||||
describe('useModelProvider - displayName functionality', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
localStorageMock.getItem.mockReturnValue(null)
|
||||
|
||||
// Reset Zustand store to default state
|
||||
act(() => {
|
||||
useModelProvider.setState({
|
||||
providers: [],
|
||||
selectedProvider: 'llamacpp',
|
||||
selectedModel: null,
|
||||
deletedModels: [],
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
it('should handle models without displayName property', () => {
|
||||
const { result } = renderHook(() => useModelProvider())
|
||||
|
||||
const provider = {
|
||||
provider: 'llamacpp',
|
||||
active: true,
|
||||
models: [
|
||||
{
|
||||
id: 'test-model.gguf',
|
||||
capabilities: ['completion'],
|
||||
},
|
||||
],
|
||||
settings: [],
|
||||
} as any
|
||||
|
||||
// First add the provider, then update it (since updateProvider only updates existing providers)
|
||||
act(() => {
|
||||
result.current.addProvider(provider)
|
||||
})
|
||||
|
||||
const updatedProvider = result.current.getProviderByName('llamacpp')
|
||||
expect(updatedProvider?.models[0].displayName).toBeUndefined()
|
||||
expect(updatedProvider?.models[0].id).toBe('test-model.gguf')
|
||||
})
|
||||
|
||||
it('should preserve displayName when merging providers in setProviders', () => {
|
||||
const { result } = renderHook(() => useModelProvider())
|
||||
|
||||
// First, set up initial state with displayName via direct state manipulation
|
||||
// This simulates the scenario where a user has already customized a display name
|
||||
act(() => {
|
||||
useModelProvider.setState({
|
||||
providers: [
|
||||
{
|
||||
provider: 'llamacpp',
|
||||
active: true,
|
||||
models: [
|
||||
{
|
||||
id: 'test-model.gguf',
|
||||
displayName: 'My Custom Model',
|
||||
capabilities: ['completion'],
|
||||
},
|
||||
],
|
||||
settings: [],
|
||||
},
|
||||
] as any,
|
||||
selectedProvider: 'llamacpp',
|
||||
selectedModel: null,
|
||||
deletedModels: [],
|
||||
})
|
||||
})
|
||||
|
||||
// Now simulate setProviders with fresh data (like from server refresh)
|
||||
const freshProviders = [
|
||||
{
|
||||
provider: 'llamacpp',
|
||||
active: true,
|
||||
persist: true,
|
||||
models: [
|
||||
{
|
||||
id: 'test-model.gguf',
|
||||
capabilities: ['completion'],
|
||||
// Note: no displayName in fresh data
|
||||
},
|
||||
],
|
||||
settings: [],
|
||||
},
|
||||
] as any
|
||||
|
||||
act(() => {
|
||||
result.current.setProviders(freshProviders)
|
||||
})
|
||||
|
||||
// The displayName should be preserved from existing state
|
||||
const provider = result.current.getProviderByName('llamacpp')
|
||||
expect(provider?.models[0].displayName).toBe('My Custom Model')
|
||||
})
|
||||
|
||||
it('should provide basic functionality without breaking existing behavior', () => {
|
||||
const { result } = renderHook(() => useModelProvider())
|
||||
|
||||
// Test that basic provider operations work
|
||||
expect(result.current.providers).toEqual([])
|
||||
expect(result.current.selectedProvider).toBe('llamacpp')
|
||||
expect(result.current.selectedModel).toBeNull()
|
||||
|
||||
// Test addProvider functionality
|
||||
const provider = {
|
||||
provider: 'openai',
|
||||
active: true,
|
||||
models: [],
|
||||
settings: [],
|
||||
} as any
|
||||
|
||||
act(() => {
|
||||
result.current.addProvider(provider)
|
||||
})
|
||||
|
||||
expect(result.current.providers).toHaveLength(1)
|
||||
expect(result.current.getProviderByName('openai')).toBeDefined()
|
||||
})
|
||||
|
||||
it('should handle provider operations with models that have displayName', () => {
|
||||
const { result } = renderHook(() => useModelProvider())
|
||||
|
||||
// Test that we can at least get and set providers with displayName models
|
||||
const providerWithDisplayName = {
|
||||
provider: 'llamacpp',
|
||||
active: true,
|
||||
models: [
|
||||
{
|
||||
id: 'test-model.gguf',
|
||||
displayName: 'Custom Model Name',
|
||||
capabilities: ['completion'],
|
||||
},
|
||||
],
|
||||
settings: [],
|
||||
} as any
|
||||
|
||||
// Set the state directly (simulating what would happen in real usage)
|
||||
act(() => {
|
||||
useModelProvider.setState({
|
||||
providers: [providerWithDisplayName],
|
||||
selectedProvider: 'llamacpp',
|
||||
selectedModel: null,
|
||||
deletedModels: [],
|
||||
})
|
||||
})
|
||||
|
||||
const provider = result.current.getProviderByName('llamacpp')
|
||||
expect(provider?.models[0].displayName).toBe('Custom Model Name')
|
||||
expect(provider?.models[0].id).toBe('test-model.gguf')
|
||||
})
|
||||
})
|
||||
@ -104,6 +104,7 @@ export const useModelProvider = create<ModelProviderState>()(
|
||||
...model,
|
||||
settings: settings,
|
||||
capabilities: existingModel?.capabilities || model.capabilities,
|
||||
displayName: existingModel?.displayName || model.displayName,
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
@ -6,6 +6,7 @@ import {
|
||||
toGigabytes,
|
||||
formatMegaBytes,
|
||||
formatDuration,
|
||||
getModelDisplayName,
|
||||
} from '../utils'
|
||||
|
||||
describe('getProviderLogo', () => {
|
||||
@ -200,3 +201,52 @@ describe('formatDuration', () => {
|
||||
expect(formatDuration(start, 86400000)).toBe('1d 0h 0m 0s') // exactly 1 day
|
||||
})
|
||||
})
|
||||
|
||||
describe('getModelDisplayName', () => {
|
||||
it('returns displayName when it exists', () => {
|
||||
const model = {
|
||||
id: 'llama-3.2-1b-instruct-q4_k_m.gguf',
|
||||
displayName: 'My Custom Model',
|
||||
} as Model
|
||||
expect(getModelDisplayName(model)).toBe('My Custom Model')
|
||||
})
|
||||
|
||||
it('returns model.id when displayName is undefined', () => {
|
||||
const model = {
|
||||
id: 'llama-3.2-1b-instruct-q4_k_m.gguf',
|
||||
} as Model
|
||||
expect(getModelDisplayName(model)).toBe('llama-3.2-1b-instruct-q4_k_m.gguf')
|
||||
})
|
||||
|
||||
it('returns model.id when displayName is empty string', () => {
|
||||
const model = {
|
||||
id: 'llama-3.2-1b-instruct-q4_k_m.gguf',
|
||||
displayName: '',
|
||||
} as Model
|
||||
expect(getModelDisplayName(model)).toBe('llama-3.2-1b-instruct-q4_k_m.gguf')
|
||||
})
|
||||
|
||||
it('returns model.id when displayName is null', () => {
|
||||
const model = {
|
||||
id: 'llama-3.2-1b-instruct-q4_k_m.gguf',
|
||||
displayName: null as any,
|
||||
} as Model
|
||||
expect(getModelDisplayName(model)).toBe('llama-3.2-1b-instruct-q4_k_m.gguf')
|
||||
})
|
||||
|
||||
it('handles models with complex display names', () => {
|
||||
const model = {
|
||||
id: 'very-long-model-file-name-with-lots-of-details.gguf',
|
||||
displayName: 'Short Name 🤖',
|
||||
} as Model
|
||||
expect(getModelDisplayName(model)).toBe('Short Name 🤖')
|
||||
})
|
||||
|
||||
it('handles models with special characters in displayName', () => {
|
||||
const model = {
|
||||
id: 'model.gguf',
|
||||
displayName: 'Model (Version 2.0) - Fine-tuned',
|
||||
} as Model
|
||||
expect(getModelDisplayName(model)).toBe('Model (Version 2.0) - Fine-tuned')
|
||||
})
|
||||
})
|
||||
|
||||
@ -7,7 +7,6 @@ export function cn(...inputs: ClassValue[]) {
|
||||
return twMerge(clsx(inputs))
|
||||
}
|
||||
|
||||
|
||||
export function basenameNoExt(filePath: string): string {
|
||||
const base = path.basename(filePath);
|
||||
const VALID_EXTENSIONS = [".tar.gz", ".zip"];
|
||||
@ -23,6 +22,13 @@ export function basenameNoExt(filePath: string): string {
|
||||
return base.slice(0, -path.extname(base).length);
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the display name for a model, falling back to the model ID if no display name is set
|
||||
*/
|
||||
export function getModelDisplayName(model: Model): string {
|
||||
return model.displayName || model.id
|
||||
}
|
||||
|
||||
export function getProviderLogo(provider: string) {
|
||||
switch (provider) {
|
||||
case 'jan':
|
||||
|
||||
@ -3,7 +3,7 @@ import { Card, CardItem } from '@/containers/Card'
|
||||
import HeaderPage from '@/containers/HeaderPage'
|
||||
import SettingsMenu from '@/containers/SettingsMenu'
|
||||
import { useModelProvider } from '@/hooks/useModelProvider'
|
||||
import { cn, getProviderTitle } from '@/lib/utils'
|
||||
import { cn, getProviderTitle, getModelDisplayName } from '@/lib/utils'
|
||||
import {
|
||||
createFileRoute,
|
||||
Link,
|
||||
@ -777,7 +777,7 @@ function ProviderDetail() {
|
||||
className="font-medium line-clamp-1"
|
||||
title={model.id}
|
||||
>
|
||||
{model.id}
|
||||
{getModelDisplayName(model)}
|
||||
</h1>
|
||||
<Capabilities capabilities={capabilities} />
|
||||
</div>
|
||||
|
||||
@ -1,7 +1,7 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import { DefaultModelsService } from '../models/default'
|
||||
import type { HuggingFaceRepo, CatalogModel } from '../models/types'
|
||||
import { EngineManager, Model } from '@janhq/core'
|
||||
import { EngineManager } from '@janhq/core'
|
||||
|
||||
// Mock EngineManager
|
||||
vi.mock('@janhq/core', () => ({
|
||||
@ -131,18 +131,19 @@ describe('DefaultModelsService', () => {
|
||||
expect(mockEngine.update).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('should update model when modelId differs from model.id', async () => {
|
||||
it('should handle model when modelId differs from model.id', async () => {
|
||||
const modelId = 'old-model-id'
|
||||
const model = {
|
||||
id: 'new-model-id',
|
||||
settings: [{ key: 'temperature', value: 0.7 }],
|
||||
}
|
||||
mockEngine.update.mockResolvedValue(undefined)
|
||||
|
||||
await modelsService.updateModel(modelId, model as any)
|
||||
|
||||
expect(mockEngine.updateSettings).toHaveBeenCalledWith(model.settings)
|
||||
expect(mockEngine.update).toHaveBeenCalledWith(modelId, model)
|
||||
// Note: Model ID updates are now handled at the provider level in the frontend
|
||||
// The engine no longer has an update method for model metadata
|
||||
expect(mockEngine.update).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@ -163,15 +163,14 @@ export class DefaultModelsService implements ModelsService {
|
||||
}
|
||||
|
||||
async updateModel(modelId: string, model: Partial<CoreModel>): Promise<void> {
|
||||
if (model.settings)
|
||||
if (model.settings) {
|
||||
this.getEngine()?.updateSettings(
|
||||
model.settings as SettingComponentProps[]
|
||||
)
|
||||
if (modelId !== model.id) {
|
||||
await this.getEngine()
|
||||
?.update(modelId, model)
|
||||
.then(() => console.log('Model updated successfully'))
|
||||
}
|
||||
// Note: Model name/ID updates are handled at the provider level in the frontend
|
||||
// The engine doesn't have an update method for model metadata
|
||||
console.log('Model update request processed for modelId:', modelId)
|
||||
}
|
||||
|
||||
async pullModel(
|
||||
|
||||
1
web-app/src/types/modelProviders.d.ts
vendored
1
web-app/src/types/modelProviders.d.ts
vendored
@ -28,6 +28,7 @@ type Model = {
|
||||
id: string
|
||||
model?: string
|
||||
name?: string
|
||||
displayName?: string
|
||||
version?: number | string
|
||||
description?: string
|
||||
format?: string
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user