refactor: clean model selector and add more tests

web-app/src/containers/ModelCombobox.tsx

@@ -6,6 +6,216 @@ import { IconChevronDown, IconLoader2, IconRefresh } from '@tabler/icons-react'
 import { cn } from '@/lib/utils'
 import { useTranslation } from '@/i18n/react-i18next-compat'
 
+// Hook for the dropdown position
+function useDropdownPosition(open: boolean, containerRef: React.RefObject<HTMLDivElement | null>) {
+  const [dropdownPosition, setDropdownPosition] = useState({ top: 0, left: 0, width: 0 })
+
+  const updateDropdownPosition = useCallback(() => {
+    if (containerRef.current) {
+      const rect = containerRef.current.getBoundingClientRect()
+      setDropdownPosition({
+        top: rect.bottom + window.scrollY + 4,
+        left: rect.left + window.scrollX,
+        width: rect.width,
+      })
+    }
+  }, [containerRef])
+
+  // Update the position when the dropdown opens
+  useEffect(() => {
+    if (open) {
+      requestAnimationFrame(() => {
+        updateDropdownPosition()
+      })
+    }
+  }, [open, updateDropdownPosition])
+
+  // Update the position when the window is resized
+  useEffect(() => {
+    if (!open) return
+
+    const handleResize = () => {
+      updateDropdownPosition()
+    }
+
+    window.addEventListener('resize', handleResize)
+    window.addEventListener('scroll', handleResize)
+
+    return () => {
+      window.removeEventListener('resize', handleResize)
+      window.removeEventListener('scroll', handleResize)
+    }
+  }, [open, updateDropdownPosition])
+
+  return { dropdownPosition, updateDropdownPosition }
+}
+
+// Components for the different sections of the dropdown
+const ErrorSection = ({ error, onRefresh, t }: { error: string; onRefresh?: () => void; t: (key: string) => string }) => (
+  <div className="px-3 py-2 text-sm text-destructive">
+    <div className="flex items-center justify-between">
+      <span className="text-destructive font-medium">{t('common:failedToLoadModels')}</span>
+      {onRefresh && (
+        <Button
+          variant="link"
+          size="sm"
+          onClick={(e) => {
+            e.stopPropagation()
+            onRefresh?.()
+          }}
+          className="h-6 w-6 p-0 no-underline hover:bg-main-view-fg/10 text-main-view-fg"
+          aria-label="Refresh models"
+        >
+          <IconRefresh className="h-3 w-3" />
+        </Button>
+      )}
+    </div>
+    <div className="text-xs text-main-view-fg/50 mt-0">{error}</div>
+  </div>
+)
+
+const LoadingSection = ({ t }: { t: (key: string) => string }) => (
+  <div className="flex items-center justify-center px-3 py-3 text-sm text-main-view-fg/50">
+    <IconLoader2 className="h-4 w-4 animate-spin mr-2 text-main-view-fg/50" />
+    <span className="text-sm text-main-view-fg/50">{t('common:loading')}</span>
+  </div>
+)
+
+const EmptySection = ({ inputValue, t }: { inputValue: string; t: (key: string, options?: Record<string, string>) => string }) => (
+  <div className="px-3 py-3 text-sm text-main-view-fg/50 text-center">
+    {inputValue.trim() ? (
+      <span className="text-main-view-fg/50">{t('common:noModelsFoundFor', { searchValue: inputValue })}</span>
+    ) : (
+      <span className="text-main-view-fg/50">{t('common:noModels')}</span>
+    )}
+  </div>
+)
+
+const ModelsList = ({
+  filteredModels,
+  value,
+  highlightedIndex,
+  onModelSelect,
+  onHighlight
+}: {
+  filteredModels: string[]
+  value: string
+  highlightedIndex: number
+  onModelSelect: (model: string) => void
+  onHighlight: (index: number) => void
+}) => (
+  <>
+    {filteredModels.map((model, index) => (
+      <div
+        key={model}
+        data-model={model}
+        onClick={(e) => {
+          e.stopPropagation()
+          onModelSelect(model)
+        }}
+        onMouseEnter={() => onHighlight(index)}
+        className={cn(
+          'cursor-pointer px-3 py-2 hover:bg-main-view-fg/15 hover:shadow-sm transition-all duration-200 text-main-view-fg',
+          value === model && 'bg-main-view-fg/12 shadow-sm',
+          highlightedIndex === index && 'bg-main-view-fg/20 shadow-md'
+        )}
+      >
+        <span className="text-sm truncate text-main-view-fg">{model}</span>
+      </div>
+    ))}
+  </>
+)
+
+// Custom hook for keyboard navigation
+function useKeyboardNavigation(
+  open: boolean,
+  setOpen: React.Dispatch<React.SetStateAction<boolean>>,
+  models: string[],
+  filteredModels: string[],
+  highlightedIndex: number,
+  setHighlightedIndex: React.Dispatch<React.SetStateAction<number>>,
+  onModelSelect: (model: string) => void,
+  dropdownRef: React.RefObject<HTMLDivElement | null>
+) {
+  const keyRepeatTimeoutRef = useRef<NodeJS.Timeout | null>(null)
+
+  // Scroll to the highlighted element
+  useEffect(() => {
+    if (highlightedIndex >= 0 && dropdownRef.current) {
+      requestAnimationFrame(() => {
+        const modelElements = dropdownRef.current?.querySelectorAll('[data-model]')
+        const highlightedElement = modelElements?.[highlightedIndex] as HTMLElement
+        if (highlightedElement) {
+          highlightedElement.scrollIntoView({
+            block: 'nearest',
+            behavior: 'auto'
+          })
+        }
+      })
+    }
+  }, [highlightedIndex, dropdownRef])
+
+  const handleKeyDown = useCallback((e: React.KeyboardEvent) => {
+    // Open the dropdown with the arrows if closed
+    if (!open && (e.key === 'ArrowDown' || e.key === 'ArrowUp')) {
+      if (models.length > 0) {
+        e.preventDefault()
+        setOpen(true)
+        setHighlightedIndex(0)
+      }
+      return
+    }
+
+    if (!open) return
+
+    switch (e.key) {
+      case 'ArrowDown':
+        e.preventDefault()
+        if (keyRepeatTimeoutRef.current) {
+          clearTimeout(keyRepeatTimeoutRef.current)
+        }
+        setHighlightedIndex((prev: number) => filteredModels.length === 0 ? 0 : (prev < filteredModels.length - 1 ? prev + 1 : 0))
+        break
+      case 'ArrowUp':
+        e.preventDefault()
+        if (keyRepeatTimeoutRef.current) {
+          clearTimeout(keyRepeatTimeoutRef.current)
+        }
+        setHighlightedIndex((prev: number) => filteredModels.length === 0 ? 0 : (prev > 0 ? prev - 1 : filteredModels.length - 1))
+        break
+      case 'Enter':
+        e.preventDefault()
+        if (highlightedIndex >= 0 && highlightedIndex < filteredModels.length) {
+          onModelSelect(filteredModels[highlightedIndex])
+        }
+        break
+      case 'ArrowRight':
+      case 'ArrowLeft':
+        setOpen(false)
+        setHighlightedIndex(-1)
+        break
+      case 'PageUp':
+        setHighlightedIndex(0)
+        break
+      case 'PageDown':
+        setHighlightedIndex(filteredModels.length - 1)
+        break
+    }
+  }, [open, setOpen, models.length, filteredModels, highlightedIndex, setHighlightedIndex, onModelSelect])
+
+  // Cleanup the timeout
+  useEffect(() => {
+    const timeoutId = keyRepeatTimeoutRef.current
+    return () => {
+      if (timeoutId) {
+        clearTimeout(timeoutId)
+      }
+    }
+  }, [])
+
+  return { handleKeyDown }
+}
+
 type ModelComboboxProps = {
   value: string
   onChange: (value: string) => void
@@ -31,12 +241,10 @@ export function ModelCombobox({
 }: ModelComboboxProps) {
   const [open, setOpen] = useState(false)
   const [inputValue, setInputValue] = useState(value)
-  const [dropdownPosition, setDropdownPosition] = useState({ top: 0, left: 0, width: 0 })
   const [highlightedIndex, setHighlightedIndex] = useState(-1)
   const inputRef = useRef<HTMLInputElement>(null)
   const containerRef = useRef<HTMLDivElement>(null)
-  const dropdownRef = useRef<HTMLDivElement>(null)
+  const dropdownRef = useRef<HTMLDivElement | null>(null)
-  const keyRepeatTimeoutRef = useRef<NodeJS.Timeout | null>(null)
   const { t } = useTranslation()
 
   // Sync input value with prop value
@@ -44,47 +252,36 @@ export function ModelCombobox({
     setInputValue(value)
   }, [value])
 
-  // Simple position calculation
+  // Hook for the dropdown position
-  const updateDropdownPosition = useCallback(() => {
+  const { dropdownPosition } = useDropdownPosition(open, containerRef)
-    if (containerRef.current) {
-      const rect = containerRef.current.getBoundingClientRect()
-      setDropdownPosition({
-        top: rect.bottom + window.scrollY + 4,
-        left: rect.left + window.scrollX,
-        width: rect.width,
-      })
-    }
-  }, [])
 
-  // Update position when opening
+  // Optimized model filtering
+  const filteredModels = useMemo(() => {
+    if (!inputValue.trim()) return models
+    const searchValue = inputValue.toLowerCase()
+    return models.filter((model) => model.toLowerCase().includes(searchValue))
+  }, [models, inputValue])
+
+  // Reset highlighted index when filtered models change
   useEffect(() => {
-    if (open) {
+    setHighlightedIndex(-1)
-      // Use requestAnimationFrame to ensure DOM is ready
+  }, [filteredModels])
-      requestAnimationFrame(() => {
-        updateDropdownPosition()
-      })
-    }
-  }, [open, updateDropdownPosition])
 
-  // Close dropdown when clicking outside
+  // Close the dropdown when clicking outside
   useEffect(() => {
     if (!open) return
 
     const handleClickOutside = (event: Event) => {
       const target = event.target as Node
-      // Check if click is inside our container or dropdown
       const isInsideContainer = containerRef.current?.contains(target)
       const isInsideDropdown = dropdownRef.current?.contains(target)
 
-      // Only close if click is outside both container and dropdown
       if (!isInsideContainer && !isInsideDropdown) {
         setOpen(false)
-        setDropdownPosition({ top: 0, left: 0, width: 0 })
         setHighlightedIndex(-1)
       }
     }
 
-    // Use multiple event types to ensure we catch all interactions
     const events = ['mousedown', 'touchstart']
     events.forEach(eventType => {
       document.addEventListener(eventType, handleClickOutside, { capture: true, passive: true })
@@ -97,127 +294,60 @@ export function ModelCombobox({
     }
   }, [open])
 
-  // Cleanup: close dropdown when component unmounts
+  // Cleanup: close the dropdown when the component is unmounted
   useEffect(() => {
-    const timeoutId = keyRepeatTimeoutRef.current
     return () => {
       setOpen(false)
-      setDropdownPosition({ top: 0, left: 0, width: 0 })
       setHighlightedIndex(-1)
-      if (timeoutId) {
-        clearTimeout(timeoutId)
-      }
     }
   }, [])
 
-    // Filter models based on input
+  // Handler for the input change
-  const filteredModels = useMemo(() => {
+  const handleInputChange = useCallback((newValue: string) => {
-    if (!inputValue.trim()) return models
-    
-    return models.filter((model) =>
-      model.toLowerCase().includes(inputValue.toLowerCase())
-    )
-  }, [models, inputValue])
-
-  // Reset highlighted index when filtered models change
-  useEffect(() => {
-    setHighlightedIndex(-1)
-  }, [filteredModels])
-
-  // Scroll to highlighted item with debouncing to handle key repeat
-  useEffect(() => {
-    if (highlightedIndex >= 0 && dropdownRef.current && !loading && !error) {
-      // Use requestAnimationFrame to ensure smooth scrolling and avoid conflicts
-      requestAnimationFrame(() => {
-        // Find all model elements (they have the data-model attribute)
-        const modelElements = dropdownRef.current?.querySelectorAll('[data-model]')
-        const highlightedElement = modelElements?.[highlightedIndex] as HTMLElement
-        if (highlightedElement) {
-          highlightedElement.scrollIntoView({
-            block: 'nearest',
-            behavior: 'auto'
-          })
-        }
-      })
-    }
-  }, [highlightedIndex, error, loading])
-
-    // Handle input change
-  const handleInputChange = (newValue: string) => {
     setInputValue(newValue)
     onChange(newValue)
 
-    // Only open dropdown if user is actively typing and there are models
+    // Open the dropdown if the user types and there are models
     if (newValue.trim() && models.length > 0) {
       setOpen(true)
     } else {
-      // Don't auto-open on empty input - wait for user interaction
       setOpen(false)
     }
-  }
+  }, [onChange, models.length])
 
-  // Handle keyboard navigation
+  // Handler for the model selection
-  const handleKeyDown = (e: React.KeyboardEvent) => {
+  const handleModelSelect = useCallback((model: string) => {
-    if (!open && (e.key === 'ArrowDown' || e.key === 'ArrowUp')) {
-      // Open dropdown on arrow keys if there are models
-      if (models.length > 0) {
-        e.preventDefault()
-        setOpen(true)
-        setHighlightedIndex(0)
-      }
-      return
-    }
-
-    if (!open) return
-
-    switch (e.key) {
-      case 'ArrowDown':
-        e.preventDefault()
-        if (keyRepeatTimeoutRef.current) {
-          clearTimeout(keyRepeatTimeoutRef.current)
-        }
-        setHighlightedIndex((prev) => 
-          filteredModels.length === 0 ? 0 : (prev < filteredModels.length - 1 ? prev + 1 : 0)
-        )
-        break
-      case 'ArrowUp':
-        e.preventDefault()
-        if (keyRepeatTimeoutRef.current) {
-          clearTimeout(keyRepeatTimeoutRef.current)
-        }
-        setHighlightedIndex((prev) => 
-          filteredModels.length === 0 ? 0 : (prev > 0 ? prev - 1 : filteredModels.length - 1)
-        )
-        break
-      case 'Enter':
-        e.preventDefault()
-        if (highlightedIndex >= 0 && highlightedIndex < filteredModels.length) {
-          handleModelSelect(filteredModels[highlightedIndex])
-        }
-        break
-      case 'ArrowRight':
-      case 'ArrowLeft':
-        setOpen(false)
-        setHighlightedIndex(-1)
-        break
-      case 'PageUp':
-        setHighlightedIndex(0)
-        break
-      case 'PageDown':
-        setHighlightedIndex(filteredModels.length - 1)
-        break
-    }
-  }
-
-  // Handle model selection from dropdown
-  const handleModelSelect = (model: string) => {
     setInputValue(model)
     onChange(model)
     setOpen(false)
-    setDropdownPosition({ top: 0, left: 0, width: 0 })
     setHighlightedIndex(-1)
     inputRef.current?.focus()
-  }
+  }, [onChange])
+
+  // Hook for the keyboard navigation
+  const { handleKeyDown } = useKeyboardNavigation(
+    open,
+    setOpen,
+    models,
+    filteredModels,
+    highlightedIndex,
+    setHighlightedIndex,
+    handleModelSelect,
+    dropdownRef
+  )
+
+  // Handler for the dropdown opening
+  const handleDropdownToggle = useCallback(() => {
+    inputRef.current?.focus()
+    setOpen(!open)
+  }, [open])
+
+  // Handler for the input click
+  const handleInputClick = useCallback(() => {
+    if (models.length > 0) {
+      setOpen(true)
+    }
+  }, [models.length])
 
   return (
     <div className={cn('relative', className)} ref={containerRef}>
@@ -227,12 +357,7 @@ export function ModelCombobox({
           value={inputValue}
           onChange={(e) => handleInputChange(e.target.value)}
           onKeyDown={handleKeyDown}
-          onClick={() => {
+          onClick={handleInputClick}
-            // Open dropdown on click if models are available
-            if (models.length > 0) {
-              setOpen(true)
-            }
-          }}
           placeholder={placeholder}
           disabled={disabled}
           className="pr-8"
@@ -243,14 +368,8 @@ export function ModelCombobox({
           variant="link"
           size="sm"
           disabled={disabled}
-          onMouseDown={(e) => {
+          onMouseDown={(e) => e.preventDefault()}
-            // Prevent losing focus from input
+          onClick={handleDropdownToggle}
-            e.preventDefault()
-          }}
-          onClick={() => {
-            inputRef.current?.focus()
-            setOpen(!open)
-          }}
           className="absolute right-1 top-1/2 h-6 w-6 p-0 -translate-y-1/2 no-underline hover:bg-main-view-fg/10"
         >
           {loading ? (
@@ -274,89 +393,30 @@ export function ModelCombobox({
               pointerEvents: 'auto',
             }}
             data-dropdown="model-combobox"
-            onPointerDown={(e) => {
+            onPointerDown={(e) => e.stopPropagation()}
-              // Prevent interaction with underlying elements
+            onClick={(e) => e.stopPropagation()}
-              e.stopPropagation()
+            onMouseDown={(e) => e.stopPropagation()}
-            }}
+            onWheel={(e) => e.stopPropagation()}
-            onClick={(e) => {
-              // Prevent click from bubbling up and closing modal
-              e.stopPropagation()
-            }}
-            onMouseDown={(e) => {
-              // Allow default behavior for scrolling and selection
-              e.stopPropagation()
-            }}
-            onWheel={(e) => {
-              // Allow wheel events for scrolling
-              e.stopPropagation()
-            }}
           >
             {/* Error state */}
-            {error && (
+            {error && <ErrorSection error={error} onRefresh={onRefresh} t={t} />}
-              <div className="px-3 py-2 text-sm text-destructive">
-                <div className="flex items-center justify-between">
-                  <span className="text-destructive font-medium">{t('common:failedToLoadModels')}</span>
-                  {onRefresh && (
-                    <Button
-                      variant="link"
-                      size="sm"
-                      onClick={(e) => {
-                        e.stopPropagation()
-                        onRefresh?.()
-                      }}
-                      className="h-6 w-6 p-0 no-underline hover:bg-main-view-fg/10 text-main-view-fg"
-                    >
-                      <IconRefresh className="h-3 w-3" />
-                    </Button>
-                  )}
-                </div>
-                <div className="text-xs text-main-view-fg/50 mt-0">{error}</div>
-              </div>
-            )}
 
             {/* Loading state */}
-            {loading && (
+            {loading && <LoadingSection t={t} />}
-              <div className="flex items-center justify-center px-3 py-3 text-sm text-main-view-fg/50">
-                <IconLoader2 className="h-4 w-4 animate-spin mr-2 text-main-view-fg/50" />
-                <span className="text-sm text-main-view-fg/50">{t('common:loading')}</span>
-              </div>
-            )}
 
             {/* Models list */}
             {!loading && !error && (
-              <>
+              filteredModels.length === 0 ? (
-                {filteredModels.length === 0 ? (
+                <EmptySection inputValue={inputValue} t={t} />
-                  <div className="px-3 py-3 text-sm text-main-view-fg/50 text-center">
+              ) : (
-                    {inputValue.trim() ? (
+                <ModelsList
-                      <span className="text-main-view-fg/50">{t('common:noModelsFoundFor', { searchValue: inputValue })}</span>
+                  filteredModels={filteredModels}
-                    ) : (
+                  value={value}
-                      <span className="text-main-view-fg/50">{t('common:noModels')}</span>
+                  highlightedIndex={highlightedIndex}
-                    )}
+                  onModelSelect={handleModelSelect}
-                  </div>
+                  onHighlight={setHighlightedIndex}
-                ) : (
+                />
-                  <>
+              )
-                    {/* Available models */}
-                    {filteredModels.map((model, index) => (
-                      <div
-                        key={model}
-                        data-model={model}
-                        onClick={(e) => {
-                          e.stopPropagation()
-                          handleModelSelect(model)
-                        }}
-                        onMouseEnter={() => setHighlightedIndex(index)}
-                        className={cn(
-                          'cursor-pointer px-3 py-2 hover:bg-main-view-fg/15 hover:shadow-sm transition-all duration-200 text-main-view-fg',
-                          value === model && 'bg-main-view-fg/12 shadow-sm',
-                          highlightedIndex === index && 'bg-main-view-fg/20 shadow-md'
-                        )}
-                      >
-                        <span className="text-sm truncate text-main-view-fg">{model}</span>
-                      </div>
-                    ))}
-                  </>
-                )}
-              </>
             )}
           </div>,
           document.body

web-app/src/containers/__tests__/ModelCombobox.test.tsx

@@ -1,197 +1,509 @@
-import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
+import { describe, it, expect, vi, beforeEach } from 'vitest'
 import { render, screen, fireEvent, waitFor, act } from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 import '@testing-library/jest-dom/vitest'
-import { ModelCombobox } from '../ModelCombobox'
 import React from 'react'
+import { ModelCombobox } from '../ModelCombobox'
+
+// Mock translation hook
+vi.mock('@/i18n/react-i18next-compat', () => ({
+  useTranslation: () => ({
+    t: (key: string, options?: Record<string, string>) => {
+      if (key === 'common:failedToLoadModels') return 'Failed to load models'
+      if (key === 'common:loading') return 'Loading'
+      if (key === 'common:noModelsFoundFor') return `No models found for "${options?.searchValue}"`
+      if (key === 'common:noModels') return 'No models available'
+      return key
+    },
+  }),
+}))
 
 describe('ModelCombobox', () => {
+  const mockOnChange = vi.fn()
+  const mockOnRefresh = vi.fn()
+  
   const defaultProps = {
     value: '',
-    onChange: vi.fn(),
+    onChange: mockOnChange,
     models: ['gpt-3.5-turbo', 'gpt-4', 'claude-3-haiku'],
   }
 
-  const mockUser = userEvent.setup()
-
   beforeEach(() => {
     vi.clearAllMocks()
+
+    Element.prototype.getBoundingClientRect = vi.fn(() => ({
+      width: 300,
+      height: 40,
+      top: 100,
+      left: 50,
+      bottom: 140,
+      right: 350,
+      x: 50,
+      y: 100,
+      toJSON: () => {},
+    }))
+    
+    Element.prototype.scrollIntoView = vi.fn()
   })
 
-  afterEach(() => {
+  it('renders input field with default placeholder', () => {
-    vi.restoreAllMocks()
+    act(() => {
-  })
-
-  describe('Basic Rendering', () => {
-    it('should render input field with placeholder', () => {
       render(<ModelCombobox {...defaultProps} />)
-
-      const input = screen.getByRole('textbox')
-      expect(input).toBeInTheDocument()
-      expect(input).toHaveAttribute('placeholder', 'Type or select a model...')
     })
     
-    it('should render custom placeholder', () => {
+    const input = screen.getByRole('textbox')
+    expect(input).toBeInTheDocument()
+    expect(input).toHaveAttribute('placeholder', 'Type or select a model...')
+  })
+
+  it('renders custom placeholder', () => {
+    act(() => {
       render(<ModelCombobox {...defaultProps} placeholder="Choose a model" />)
-
-      const input = screen.getByRole('textbox')
-      expect(input).toHaveAttribute('placeholder', 'Choose a model')
     })
     
-    it('should render dropdown trigger button', () => {
+    const input = screen.getByRole('textbox')
+    expect(input).toHaveAttribute('placeholder', 'Choose a model')
+  })
+
+  it('renders dropdown trigger button', () => {
+    act(() => {
       render(<ModelCombobox {...defaultProps} />)
-      const button = screen.getByRole('button')
-      expect(button).toBeInTheDocument()
     })
     
-    it('should display current value in input', () => {
+    const button = screen.getByRole('button')
+    expect(button).toBeInTheDocument()
+  })
+
+  it('displays current value in input', () => {
+    act(() => {
       render(<ModelCombobox {...defaultProps} value="gpt-4" />)
-
-      const input = screen.getByDisplayValue('gpt-4')
-      expect(input).toBeInTheDocument()
     })
     
-    it('should apply custom className', () => {
+    const input = screen.getByDisplayValue('gpt-4')
-      const { container } = render(
+    expect(input).toBeInTheDocument()
-        <ModelCombobox {...defaultProps} className="custom-class" />
-      )
-
-      const wrapper = container.firstChild as HTMLElement
-      expect(wrapper).toHaveClass('custom-class')
-    })
   })
 
-  describe('Disabled State', () => {
+  it('applies custom className', () => {
-    it('should disable input when disabled prop is true', () => {
+    const { container } = render(
+      <ModelCombobox {...defaultProps} className="custom-class" />
+    )
+
+    const wrapper = container.firstChild as HTMLElement
+    expect(wrapper).toHaveClass('custom-class')
+  })
+
+  it('disables input when disabled prop is true', () => {
+    act(() => {
       render(<ModelCombobox {...defaultProps} disabled />)
-
-      const input = screen.getByRole('textbox')
-      const button = screen.getByRole('button')
-
-      expect(input).toBeDisabled()
-      expect(button).toBeDisabled()
     })
     
-    it('should not open dropdown when disabled', async () => {
+    const input = screen.getByRole('textbox')
-      render(<ModelCombobox {...defaultProps} disabled />)
+    const button = screen.getByRole('button')
 
-      const input = screen.getByRole('textbox')
+    expect(input).toBeDisabled()
-      await mockUser.click(input)
+    expect(button).toBeDisabled()
-
-      expect(screen.queryByTestId('dropdown')).not.toBeInTheDocument()
-    })
   })
 
-  describe('Loading State', () => {
+  it('shows loading spinner in trigger button', () => {
-    it('should show loading spinner in trigger button', () => {
+    act(() => {
       render(<ModelCombobox {...defaultProps} loading />)
-
-      const button = screen.getByRole('button')
-      const spinner = button.querySelector('.animate-spin')
-      expect(spinner).toBeInTheDocument()
     })
     
-    it('should show loading spinner when loading prop is true', () => {
+    const button = screen.getByRole('button')
-      render(<ModelCombobox {...defaultProps} loading />)
+    const spinner = button.querySelector('.animate-spin')
+    expect(spinner).toBeInTheDocument()
+  })
 
-      const spinner = screen.getByRole('button').querySelector('.animate-spin')
+  it('shows loading section when dropdown is opened during loading', async () => {
-      expect(spinner).toBeInTheDocument()
+    const user = userEvent.setup()
+    render(<ModelCombobox {...defaultProps} loading />)
+    
+    // Click input to trigger dropdown opening
+    const input = screen.getByRole('textbox')
+    await user.click(input)
+    
+    // Wait for dropdown to appear and check loading section
+    await waitFor(() => {
+      const dropdown = document.querySelector('[data-dropdown="model-combobox"]')
+      expect(dropdown).toBeInTheDocument()
+      expect(screen.getByText('Loading')).toBeInTheDocument()
     })
   })
 
-  describe('Input Interactions', () => {
+  it('calls onChange when typing', async () => {
-    it('should call onChange when typing', async () => {
+    const user = userEvent.setup()
-      const mockOnChange = vi.fn()
+    const localMockOnChange = vi.fn()
-      render(<ModelCombobox {...defaultProps} onChange={mockOnChange} />)
+    render(<ModelCombobox {...defaultProps} onChange={localMockOnChange} />)
 
-      const input = screen.getByRole('textbox')
+    const input = screen.getByRole('textbox')
-      await mockUser.type(input, 'g')
+    await user.type(input, 'g')
 
-      expect(mockOnChange).toHaveBeenCalledWith('g')
+    expect(localMockOnChange).toHaveBeenCalledWith('g')
-    })
-
-    it('should update input value when typing', async () => {
-      const mockOnChange = vi.fn()
-      render(<ModelCombobox {...defaultProps} onChange={mockOnChange} />)
-
-      const input = screen.getByRole('textbox')
-      await mockUser.type(input, 'test')
-
-      expect(input).toHaveValue('test')
-    })
-
-    it('should handle input focus', async () => {
-      render(<ModelCombobox {...defaultProps} />)
-
-      const input = screen.getByRole('textbox')
-      await mockUser.click(input)
-
-      expect(input).toHaveFocus()
-    })
   })
 
-  describe('Props Validation', () => {
+  it('updates input value when typing', async () => {
-    it('should render with empty models array', () => {
+    const user = userEvent.setup()
+    render(<ModelCombobox {...defaultProps} />)
+
+    const input = screen.getByRole('textbox')
+    await user.type(input, 'test')
+
+    expect(input).toHaveValue('test')
+  })
+
+  it('handles input focus', async () => {
+    const user = userEvent.setup()
+    render(<ModelCombobox {...defaultProps} />)
+
+    const input = screen.getByRole('textbox')
+    await user.click(input)
+
+    expect(input).toHaveFocus()
+  })
+
+  it('renders with empty models array', () => {
+    act(() => {
       render(<ModelCombobox {...defaultProps} models={[]} />)
-
-      const input = screen.getByRole('textbox')
-      expect(input).toBeInTheDocument()
     })
     
-    it('should render with models array', () => {
+    const input = screen.getByRole('textbox')
+    expect(input).toBeInTheDocument()
+  })
+
+  it('renders with models array', () => {
+    act(() => {
       render(<ModelCombobox {...defaultProps} models={['model1', 'model2']} />)
-
-      const input = screen.getByRole('textbox')
-      expect(input).toBeInTheDocument()
     })
     
-    it('should render with all props', () => {
+    const input = screen.getByRole('textbox')
+    expect(input).toBeInTheDocument()
+  })
+
+  it('handles mount and unmount without errors', () => {
+    const { unmount } = render(<ModelCombobox {...defaultProps} />)
+
+    expect(screen.getByRole('textbox')).toBeInTheDocument()
+
+    unmount()
+
+    expect(screen.queryByRole('textbox')).not.toBeInTheDocument()
+  })
+
+  it('handles props changes', () => {
+    const { rerender } = render(<ModelCombobox {...defaultProps} value="" />)
+
+    expect(screen.getByDisplayValue('')).toBeInTheDocument()
+
+    rerender(<ModelCombobox {...defaultProps} value="gpt-4" />)
+
+    expect(screen.getByDisplayValue('gpt-4')).toBeInTheDocument()
+  })
+
+  it('handles models array changes', () => {
+    const { rerender } = render(<ModelCombobox {...defaultProps} models={[]} />)
+
+    expect(screen.getByRole('textbox')).toBeInTheDocument()
+
+    rerender(<ModelCombobox {...defaultProps} models={['model1', 'model2']} />)
+
+    expect(screen.getByRole('textbox')).toBeInTheDocument()
+  })
+
+  it('does not open dropdown when clicking input with no models', async () => {
+    const user = userEvent.setup()
+    render(<ModelCombobox {...defaultProps} models={[]} />)
+
+    const input = screen.getByRole('textbox')
+    await user.click(input)
+
+    // Should focus but not open dropdown
+    expect(input).toHaveFocus()
+    const dropdown = document.querySelector('[data-dropdown="model-combobox"]')
+    expect(dropdown).not.toBeInTheDocument()
+  })
+
+  it('accepts error prop without crashing', () => {
+    act(() => {
+      render(<ModelCombobox {...defaultProps} error="Test error message" />)
+    })
+
+    const input = screen.getByRole('textbox')
+    expect(input).toBeInTheDocument()
+    expect(input).toHaveAttribute('placeholder', 'Type or select a model...')
+  })
+
+  it('renders with all props', () => {
+    act(() => {
       render(
         <ModelCombobox
           {...defaultProps}
           loading
           error="Error message"
-          onRefresh={vi.fn()}
+          onRefresh={mockOnRefresh}
           placeholder="Custom placeholder"
           disabled
         />
       )
+    })
     
-      const input = screen.getByRole('textbox')
+    const input = screen.getByRole('textbox')
-      expect(input).toBeInTheDocument()
+    expect(input).toBeInTheDocument()
-      expect(input).toBeDisabled()
+    expect(input).toBeDisabled()
+  })
+
+  it('opens dropdown when clicking trigger button', async () => {
+    const user = userEvent.setup()
+    render(<ModelCombobox {...defaultProps} />)
+
+    const button = screen.getByRole('button')
+    await user.click(button)
+
+    await waitFor(() => {
+      const dropdown = document.querySelector('[data-dropdown="model-combobox"]')
+      expect(dropdown).toBeInTheDocument()
     })
   })
 
-  describe('Component Lifecycle', () => {
+  it('opens dropdown when clicking input', async () => {
-    it('should handle mount and unmount without errors', () => {
+    const user = userEvent.setup()
-      const { unmount } = render(<ModelCombobox {...defaultProps} />)
+    render(<ModelCombobox {...defaultProps} />)
 
-      expect(screen.getByRole('textbox')).toBeInTheDocument()
+    const input = screen.getByRole('textbox')
+    await user.click(input)
 
-      unmount()
+    expect(input).toHaveFocus()
+    await waitFor(() => {
+      const dropdown = document.querySelector('[data-dropdown="model-combobox"]')
+      expect(dropdown).toBeInTheDocument()
+    })
+  })
 
-      expect(screen.queryByRole('textbox')).not.toBeInTheDocument()
+  it('filters models based on input value', async () => {
+    const user = userEvent.setup()
+    const localMockOnChange = vi.fn()
+    render(<ModelCombobox {...defaultProps} onChange={localMockOnChange} />)
+
+    const input = screen.getByRole('textbox')
+    await user.type(input, 'gpt-4')
+
+    expect(localMockOnChange).toHaveBeenCalledWith('gpt-4')
+  })
+
+  it('shows filtered models in dropdown when typing', async () => {
+    const user = userEvent.setup()
+    render(<ModelCombobox {...defaultProps} />)
+
+    const input = screen.getByRole('textbox')
+    // Type 'gpt' to trigger dropdown opening
+    await user.type(input, 'gpt')
+
+    await waitFor(() => {
+      // Dropdown should be open
+      const dropdown = document.querySelector('[data-dropdown="model-combobox"]')
+      expect(dropdown).toBeInTheDocument()
+      
+      // Should show GPT models
+      expect(screen.getByText('gpt-3.5-turbo')).toBeInTheDocument()
+      expect(screen.getByText('gpt-4')).toBeInTheDocument()
+      // Should not show Claude
+      expect(screen.queryByText('claude-3-haiku')).not.toBeInTheDocument()
+    })
+  })
+
+  it('handles case insensitive filtering', async () => {
+    const user = userEvent.setup()
+    render(<ModelCombobox {...defaultProps} />)
+
+    const input = screen.getByRole('textbox')
+    await user.type(input, 'GPT')
+
+    expect(mockOnChange).toHaveBeenCalledWith('GPT')
+  })
+
+  it('shows empty state when no models match filter', async () => {
+    const user = userEvent.setup()
+    render(<ModelCombobox {...defaultProps} />)
+
+    const input = screen.getByRole('textbox')
+    // Type something that doesn't match any model to trigger dropdown + empty state
+    await user.type(input, 'nonexistent')
+
+    await waitFor(() => {
+      // Dropdown should be open
+      const dropdown = document.querySelector('[data-dropdown="model-combobox"]')
+      expect(dropdown).toBeInTheDocument()
+      // Should show empty state message
+      expect(screen.getByText('No models found for "nonexistent"')).toBeInTheDocument()
+    })
+  })
+
+  it('selects model from dropdown when clicked', async () => {
+    const user = userEvent.setup()
+    const localMockOnChange = vi.fn()
+    render(<ModelCombobox {...defaultProps} onChange={localMockOnChange} />)
+
+    const input = screen.getByRole('textbox')
+    await user.click(input)
+    
+    await waitFor(() => {
+      const modelOption = screen.getByText('gpt-4')
+      expect(modelOption).toBeInTheDocument()
     })
     
-    it('should handle props changes', () => {
+    const modelOption = screen.getByText('gpt-4')
-      const { rerender } = render(<ModelCombobox {...defaultProps} value="" />)
+    await user.click(modelOption)
 
-      expect(screen.getByDisplayValue('')).toBeInTheDocument()
+    expect(localMockOnChange).toHaveBeenCalledWith('gpt-4')
+    expect(input).toHaveValue('gpt-4')
+  })
 
-      rerender(<ModelCombobox {...defaultProps} value="gpt-4" />)
+  it('submits input value with Enter key', async () => {
+    const user = userEvent.setup()
+    const localMockOnChange = vi.fn()
+    render(<ModelCombobox {...defaultProps} onChange={localMockOnChange} />)
 
-      expect(screen.getByDisplayValue('gpt-4')).toBeInTheDocument()
+    const input = screen.getByRole('textbox')
+    await user.type(input, 'gpt')
+    await user.keyboard('{Enter}')
+
+    expect(localMockOnChange).toHaveBeenCalledWith('gpt')
+  })
+
+  it('updates input value when typing', () => {
+    render(<ModelCombobox {...defaultProps} />)
+
+    const input = screen.getByRole('textbox')
+    fireEvent.change(input, { target: { value: 'gpt-4' } })
+
+    expect(input).toHaveValue('gpt-4')
+  })
+
+  it('displays error message in dropdown', async () => {
+    const user = userEvent.setup()
+    render(<ModelCombobox {...defaultProps} error="Network connection failed" />)
+
+    const input = screen.getByRole('textbox')
+    // Click input to open dropdown
+    await user.click(input)
+
+    await waitFor(() => {
+      // Dropdown should be open
+      const dropdown = document.querySelector('[data-dropdown="model-combobox"]')
+      expect(dropdown).toBeInTheDocument()
+      // Error messages should be displayed
+      expect(screen.getByText('Failed to load models')).toBeInTheDocument()
+      expect(screen.getByText('Network connection failed')).toBeInTheDocument()
+    })
+  })
+
+  it('calls onRefresh when refresh button is clicked', async () => {
+    const user = userEvent.setup()
+    const localMockOnRefresh = vi.fn()
+    render(<ModelCombobox {...defaultProps} error="Network error" onRefresh={localMockOnRefresh} />)
+
+    const input = screen.getByRole('textbox')
+    // Click input to open dropdown
+    await user.click(input)
+
+    await waitFor(() => {
+      // Dropdown should be open with error section
+      const dropdown = document.querySelector('[data-dropdown="model-combobox"]')
+      expect(dropdown).toBeInTheDocument()
+      const refreshButton = document.querySelector('[aria-label="Refresh models"]')
+      expect(refreshButton).toBeInTheDocument()
     })
 
-    it('should handle models array changes', () => {
+    const refreshButton = document.querySelector('[aria-label="Refresh models"]')
-      const { rerender } = render(<ModelCombobox {...defaultProps} models={[]} />)
+    if (refreshButton) {
+      await user.click(refreshButton)
+      expect(localMockOnRefresh).toHaveBeenCalledTimes(1)
+    }
+  })
 
-      expect(screen.getByRole('textbox')).toBeInTheDocument()
+  it('opens dropdown when pressing ArrowDown', async () => {
+    const user = userEvent.setup()
+    render(<ModelCombobox {...defaultProps} />)
 
-      rerender(<ModelCombobox {...defaultProps} models={['model1', 'model2']} />)
+    const input = screen.getByRole('textbox')
+    input.focus()
+    await user.keyboard('{ArrowDown}')
 
-      expect(screen.getByRole('textbox')).toBeInTheDocument()
+    expect(input).toHaveFocus()
+    await waitFor(() => {
+      const dropdown = document.querySelector('[data-dropdown="model-combobox"]')
+      expect(dropdown).toBeInTheDocument()
+    })
+  })
+
+  it('navigates through models with arrow keys', async () => {
+    const user = userEvent.setup()
+    render(<ModelCombobox {...defaultProps} />)
+
+    const input = screen.getByRole('textbox')
+    input.focus()
+    
+    // ArrowDown should open dropdown
+    await user.keyboard('{ArrowDown}')
+    
+    await waitFor(() => {
+      // Dropdown should be open
+      const dropdown = document.querySelector('[data-dropdown="model-combobox"]')
+      expect(dropdown).toBeInTheDocument()
+    })
+    
+    // Navigate to second item
+    await user.keyboard('{ArrowDown}')
+
+    await waitFor(() => {
+      const secondModel = screen.getByText('gpt-4')
+      const modelElement = secondModel.closest('[data-model]')
+      expect(modelElement).toHaveClass('bg-main-view-fg/20')
+    })
+  })
+
+  it('handles Enter key to select highlighted model', async () => {
+    const user = userEvent.setup()
+    const localMockOnChange = vi.fn()
+    render(<ModelCombobox {...defaultProps} onChange={localMockOnChange} />)
+
+    const input = screen.getByRole('textbox')
+    // Type 'gpt' to open dropdown and filter models
+    await user.type(input, 'gpt')
+    
+    await waitFor(() => {
+      // Dropdown should be open with filtered models
+      const dropdown = document.querySelector('[data-dropdown="model-combobox"]')
+      expect(dropdown).toBeInTheDocument()
+    })
+    
+    // Navigate to highlight first model and select it
+    await user.keyboard('{ArrowDown}')
+    await user.keyboard('{Enter}')
+
+    expect(localMockOnChange).toHaveBeenCalledWith('gpt-3.5-turbo')
+  })
+
+  it('closes dropdown with ArrowLeft key', async () => {
+    const user = userEvent.setup()
+    render(<ModelCombobox {...defaultProps} />)
+
+    const input = screen.getByRole('textbox')
+    input.focus()
+    
+    // ArrowDown should open dropdown
+    await user.keyboard('{ArrowDown}')
+    
+    await waitFor(() => {
+      const dropdown = document.querySelector('[data-dropdown="model-combobox"]')
+      expect(dropdown).toBeInTheDocument()
+    })
+    
+    // ArrowLeft should close dropdown
+    await user.keyboard('{ArrowLeft}')
+    
+    await waitFor(() => {
+      const dropdown = document.querySelector('[data-dropdown="model-combobox"]')
+      expect(dropdown).not.toBeInTheDocument()
     })
   })
 })