Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
5d9c3ab
feat: add model selector with fetching from /v1/models endpoints when…
lugnicca Aug 20, 2025
3339629
test: add unit tests for ModelCombobox, useProviderModels and providers
lugnicca Aug 20, 2025
f35e6cd
refactor: clean model selector and add more tests
lugnicca Aug 22, 2025
9a68631
refactor: more modular error handling in fetchModelsFromProvider func…
lugnicca Aug 22, 2025
4e8dd92
refactor: simplify event handling and fix test setup in ModelCombobox
lugnicca Aug 22, 2025
1bf5802
refactor: update MockModelProvider type to use ModelProvider and clea…
lugnicca Aug 22, 2025
aa568e6
fix: remove ModelProvider type
lugnicca Aug 23, 2025
639bd5f
fix: set Escape in keyboard navigation
lugnicca Aug 23, 2025
6c0e6dc
fix: remove unused keyRepeatTimeoutRef
lugnicca Aug 23, 2025
1a6a37c
fix: escape key was closing modal instead of only combobox and remove…
lugnicca Aug 23, 2025
70bf257
fix: put refresh button directly in input instead of in dropdown
lugnicca Sep 2, 2025
3d0ce15
fix: prevent stale provider model requests from polluting UI state
lugnicca Sep 2, 2025
0457784
Merge branch 'dev' into feat/model-selector
lugnicca Sep 5, 2025
66af5c7
fix: use webprovider services to fetch models
lugnicca Sep 5, 2025
9fcd950
fix: error on message with "fetch"
lugnicca Sep 6, 2025
2db9af9
fix: use serviceHub to fetch models and fix error message on app
lugnicca Sep 8, 2025
dbcc1db
Merge branch 'dev' into feat/model-selector
lugnicca Sep 8, 2025
43431c2
Merge branch 'dev' into feat/model-selector
louis-jan Sep 15, 2025
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
430 changes: 430 additions & 0 deletions web-app/src/containers/ModelCombobox.tsx

Large diffs are not rendered by default.

490 changes: 490 additions & 0 deletions web-app/src/containers/__tests__/ModelCombobox.test.tsx

Large diffs are not rendered by default.

31 changes: 24 additions & 7 deletions web-app/src/containers/dialogs/AddModel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ import {
DialogFooter,
} from '@/components/ui/dialog'
import { Button } from '@/components/ui/button'
import { Input } from '@/components/ui/input'
import { useModelProvider } from '@/hooks/useModelProvider'
import { useProviderModels } from '@/hooks/useProviderModels'
import { ModelCombobox } from '@/containers/ModelCombobox'
import { IconPlus } from '@tabler/icons-react'
import { useState } from 'react'
import { getProviderTitle } from '@/lib/utils'
Expand All @@ -25,6 +26,12 @@ export const DialogAddModel = ({ provider, trigger }: DialogAddModelProps) => {
const { updateProvider } = useModelProvider()
const [modelId, setModelId] = useState<string>('')
const [open, setOpen] = useState(false)
const [isComboboxOpen, setIsComboboxOpen] = useState(false)

// Fetch models from provider API (API key is optional)
const { models, loading, error, refetch } = useProviderModels(
provider.base_url ? provider : undefined
)

// Handle form submission
const handleSubmit = () => {
Expand Down Expand Up @@ -62,7 +69,13 @@ export const DialogAddModel = ({ provider, trigger }: DialogAddModelProps) => {
</div>
)}
</DialogTrigger>
<DialogContent>
<DialogContent
onEscapeKeyDown={(e: KeyboardEvent) => {
if (isComboboxOpen) {
e.preventDefault()
}
}}
>
<DialogHeader>
<DialogTitle>{t('providers:addModel.title')}</DialogTitle>
<DialogDescription>
Expand All @@ -72,7 +85,7 @@ export const DialogAddModel = ({ provider, trigger }: DialogAddModelProps) => {
</DialogDescription>
</DialogHeader>

{/* Model ID field - required */}
{/* Model selection field - required */}
<div className="space-y-2">
<label
htmlFor="model-id"
Expand All @@ -81,12 +94,16 @@ export const DialogAddModel = ({ provider, trigger }: DialogAddModelProps) => {
{t('providers:addModel.modelId')}{' '}
<span className="text-destructive">*</span>
</label>
<Input
id="model-id"
<ModelCombobox
key={`${provider.provider}-${provider.base_url || ''}`}
value={modelId}
onChange={(e) => setModelId(e.target.value)}
onChange={setModelId}
models={models}
loading={loading}
error={error}
onRefresh={refetch}
placeholder={t('providers:addModel.enterModelId')}
required
onOpenChange={setIsComboboxOpen}
/>
</div>

Expand Down
103 changes: 103 additions & 0 deletions web-app/src/hooks/__tests__/useProviderModels.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import { describe, it, expect, beforeEach, vi } from 'vitest'
import { renderHook, waitFor } from '@testing-library/react'
import { useProviderModels } from '../useProviderModels'
import { WebProvidersService } from '../../services/providers/web'

let fetchModelsSpy: ReturnType<typeof vi.spyOn>

// Local minimal provider type for tests
type MockModelProvider = {
active: boolean
provider: string
base_url?: string
api_key?: string
settings: any[]
models: any[]
}

describe('useProviderModels', () => {
const mockProvider: MockModelProvider = {
active: true,
provider: 'openai',
base_url: 'https://api.openai.com/v1',
api_key: 'test-api-key',
settings: [],
models: [],
}

const mockModels = ['gpt-4', 'gpt-3.5-turbo', 'gpt-4-turbo']

beforeEach(() => {
vi.restoreAllMocks()
vi.clearAllMocks()
fetchModelsSpy = vi.spyOn(
WebProvidersService.prototype,
'fetchModelsFromProvider'
)
})

it('should initialize with empty state', () => {
const { result } = renderHook(() => useProviderModels())

expect(result.current.models).toEqual([])
expect(result.current.loading).toBe(false)
expect(result.current.error).toBe(null)
expect(typeof result.current.refetch).toBe('function')
})

it('should not fetch models when provider is undefined', () => {
renderHook(() => useProviderModels(undefined))
expect(fetchModelsSpy).not.toHaveBeenCalled()
})

it('should not fetch models when provider has no base_url', () => {
const providerWithoutUrl = { ...mockProvider, base_url: undefined }
renderHook(() => useProviderModels(providerWithoutUrl))
expect(fetchModelsSpy).not.toHaveBeenCalled()
})

it('should fetch and sort models', async () => {
fetchModelsSpy.mockResolvedValueOnce(mockModels)

const { result } = renderHook(() => useProviderModels(mockProvider))

await waitFor(() => {
expect(result.current.loading).toBe(false)
})

// Should be sorted alphabetically
expect(result.current.models).toEqual(['gpt-3.5-turbo', 'gpt-4', 'gpt-4-turbo'])
expect(result.current.error).toBe(null)
expect(fetchModelsSpy).toHaveBeenCalledWith(mockProvider)
})

it('should clear models when switching to invalid provider', async () => {
fetchModelsSpy.mockResolvedValueOnce(mockModels)

const { result, rerender } = renderHook(
({ provider }) => useProviderModels(provider),
{ initialProps: { provider: mockProvider } }
)

await waitFor(() => {
expect(result.current.loading).toBe(false)
})

expect(result.current.models).toEqual(['gpt-3.5-turbo', 'gpt-4', 'gpt-4-turbo'])

// Switch to invalid provider
rerender({ provider: { ...mockProvider, base_url: undefined } })

expect(result.current.models).toEqual([])
expect(result.current.error).toBe(null)
expect(result.current.loading).toBe(false)
})

it('should not refetch when provider is undefined', () => {
const { result } = renderHook(() => useProviderModels(undefined))

result.current.refetch()

expect(fetchModelsSpy).not.toHaveBeenCalled()
})
})
93 changes: 93 additions & 0 deletions web-app/src/hooks/useProviderModels.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import { useState, useEffect, useCallback, useRef, useMemo } from 'react'
import { WebProvidersService } from '../services/providers/web'

type UseProviderModelsState = {
models: string[]
loading: boolean
error: string | null
refetch: () => void
}

const modelsCache = new Map<string, { models: string[]; timestamp: number }>()
const CACHE_DURATION = 5 * 60 * 1000 // 5 minutes

export const useProviderModels = (provider?: ModelProvider): UseProviderModelsState => {
const providersService = useMemo(() => new WebProvidersService(), [])
const [models, setModels] = useState<string[]>([])
const [loading, setLoading] = useState(false)
const [error, setError] = useState<string | null>(null)
const prevProviderKey = useRef<string>('')
const requestIdRef = useRef(0)

const fetchModels = useCallback(async () => {
if (!provider || !provider.base_url) {
// Clear models if provider is invalid (base_url is required, api_key is optional)
setModels([])
setError(null)
setLoading(false)
return
}

// Clear any previous state when starting a new fetch for a different provider
const currentProviderKey = `${provider.provider}-${provider.base_url}`
if (currentProviderKey !== prevProviderKey.current) {
setModels([])
setError(null)
setLoading(false)
prevProviderKey.current = currentProviderKey
}

const cacheKey = `${provider.provider}-${provider.base_url}`
const cached = modelsCache.get(cacheKey)

// Check cache first
if (cached && Date.now() - cached.timestamp < CACHE_DURATION) {
setModels(cached.models)
return
}

const currentRequestId = ++requestIdRef.current
setLoading(true)
setError(null)

try {
const fetchedModels = await providersService.fetchModelsFromProvider(provider)
if (currentRequestId !== requestIdRef.current) return
const sortedModels = fetchedModels.sort((a, b) => a.localeCompare(b))

setModels(sortedModels)

// Cache the results
modelsCache.set(cacheKey, {
models: sortedModels,
timestamp: Date.now(),
})
} catch (err) {
if (currentRequestId !== requestIdRef.current) return
const errorMessage = err instanceof Error ? err.message : 'Failed to fetch models'
setError(errorMessage)
console.error(`Error fetching models from ${provider.provider}:`, err)
} finally {
if (currentRequestId === requestIdRef.current) setLoading(false)
}
}, [provider, providersService])

const refetch = useCallback(() => {
if (provider) {
const cacheKey = `${provider.provider}-${provider.base_url}`
modelsCache.delete(cacheKey)
fetchModels()
}
}, [provider, fetchModels])

useEffect(() => {
fetchModels()
}, [fetchModels])

return {
models,
loading,
error,
refetch,
}
}
2 changes: 2 additions & 0 deletions web-app/src/locales/en/common.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@
"selectAModel": "Select a model",
"noToolsAvailable": "No tools available",
"noModelsFoundFor": "No models found for \"{{searchValue}}\"",
"failedToLoadModels": "Failed to load models",
"noModels": "No models found",
"customAvatar": "Custom avatar",
"editAssistant": "Edit Assistant",
"jan": "Jan",
Expand Down
40 changes: 38 additions & 2 deletions web-app/src/services/__tests__/providers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@
)
})

it('should throw error when API response is not ok', async () => {
it('should throw error when API response is not ok (404)', async () => {
const mockResponse = {
ok: false,
status: 404,
Expand All @@ -229,7 +229,43 @@
}

await expect(providersService.fetchModelsFromProvider(provider)).rejects.toThrow(
'Cannot connect to custom at https://api.custom.com. Please check that the service is running and accessible.'
'Models endpoint not found for custom. Check the base URL configuration.'
)
})

it('should throw error when API response is not ok (403)', async () => {
const mockResponse = {
ok: false,
status: 403,
statusText: 'Forbidden',
}
vi.mocked(global.fetch).mockResolvedValue(mockResponse as any)

const provider = {
provider: 'custom',
base_url: 'https://api.custom.com',
} as ModelProvider

await expect(providersService.fetchModelsFromProvider(provider)).rejects.toThrow(
'Access forbidden: Check your API key permissions for custom'
)
})

it('should throw error when API response is not ok (401)', async () => {
const mockResponse = {
ok: false,
status: 401,
statusText: 'Unauthorized',
}
vi.mocked(global.fetch).mockResolvedValue(mockResponse as any)

const provider = {
provider: 'custom',
base_url: 'https://api.custom.com',
} as ModelProvider

await expect(providersService.fetchModelsFromProvider(provider)).rejects.toThrow(
'Authentication failed: API key is required or invalid for custom'
)
})

Expand All @@ -241,7 +277,7 @@
base_url: 'https://api.custom.com',
}

await expect(providersService.fetchModelsFromProvider(provider)).rejects.toThrow(

Check failure on line 280 in web-app/src/services/__tests__/providers.test.ts

View workflow job for this annotation

GitHub Actions / test-on-macos

src/services/__tests__/providers.test.ts > WebProvidersService > fetchModelsFromProvider > should handle network errors gracefully

AssertionError: expected [Function] to throw error including 'Cannot connect to custom at https://a…' but got 'Unexpected error while fetching model…' Expected: "Cannot connect to custom at https://api.custom.com. Please check that the service is running and accessible." Received: "Unexpected error while fetching models from custom: fetch failed" ❯ src/services/__tests__/providers.test.ts:280:7

Check failure on line 280 in web-app/src/services/__tests__/providers.test.ts

View workflow job for this annotation

GitHub Actions / test-on-windows-pr

src/services/__tests__/providers.test.ts > WebProvidersService > fetchModelsFromProvider > should handle network errors gracefully

AssertionError: expected [Function] to throw error including 'Cannot connect to custom at https://a…' but got 'Unexpected error while fetching model…' Expected: "Cannot connect to custom at https://api.custom.com. Please check that the service is running and accessible." Received: "Unexpected error while fetching models from custom: fetch failed" ❯ src/services/__tests__/providers.test.ts:280:7

Check failure on line 280 in web-app/src/services/__tests__/providers.test.ts

View workflow job for this annotation

GitHub Actions / test-on-ubuntu

src/services/__tests__/providers.test.ts > WebProvidersService > fetchModelsFromProvider > should handle network errors gracefully

AssertionError: expected [Function] to throw error including 'Cannot connect to custom at https://a…' but got 'Unexpected error while fetching model…' Expected: "Cannot connect to custom at https://api.custom.com. Please check that the service is running and accessible." Received: "Unexpected error while fetching models from custom: fetch failed" ❯ src/services/__tests__/providers.test.ts:280:7
'Cannot connect to custom at https://api.custom.com. Please check that the service is running and accessible.'
)
})
Expand Down
42 changes: 36 additions & 6 deletions web-app/src/services/providers/web.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,24 @@ export class WebProvidersService implements ProvidersService {
})

if (!response.ok) {
throw new Error(
`Cannot connect to ${provider.provider} at ${provider.base_url}. Please check that the service is running and accessible.`
)
// Provide more specific error messages based on status code
if (response.status === 401) {
throw new Error(
`Authentication failed: API key is required or invalid for ${provider.provider}`
)
} else if (response.status === 403) {
throw new Error(
`Access forbidden: Check your API key permissions for ${provider.provider}`
)
} else if (response.status === 404) {
throw new Error(
`Models endpoint not found for ${provider.provider}. Check the base URL configuration.`
)
} else {
throw new Error(
`Failed to fetch models from ${provider.provider}: ${response.status} ${response.statusText}`
)
}
}

const data = await response.json()
Expand Down Expand Up @@ -170,13 +185,28 @@ export class WebProvidersService implements ProvidersService {
} catch (error) {
console.error('Error fetching models from provider:', error)

const structuredErrorPrefixes = [
'Authentication failed',
'Access forbidden',
'Models endpoint not found',
'Failed to fetch models from'
]

if (error instanceof Error &&
structuredErrorPrefixes.some(prefix => (error as Error).message.startsWith(prefix))) {
throw new Error(error.message)
}

// Provide helpful error message for any connection errors
if (error instanceof Error && error.message.includes('Cannot connect')) {
throw error
throw new Error(
`Cannot connect to ${provider.provider} at ${provider.base_url}. Please check that the service is running and accessible.`
)
}


// Generic fallback
throw new Error(
`Cannot connect to ${provider.provider} at ${provider.base_url}. Please check that the service is running and accessible.`
`Unexpected error while fetching models from ${provider.provider}: ${error instanceof Error ? error.message : 'Unknown error'}`
)
}
}
Expand Down
Loading