Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/model-picker-sections.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"kilo-code": patch
---

Add section headers to model selection dropdowns for "Recommended models" and "All models"
71 changes: 61 additions & 10 deletions webview-ui/src/components/kilocode/chat/ModelSelector.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { useMemo } from "react"
import { SelectDropdown, DropdownOptionType } from "@/components/ui"
import { SelectDropdown, DropdownOptionType, type DropdownOption } from "@/components/ui"
import { OPENROUTER_DEFAULT_PROVIDER_NAME, type ProviderSettings } from "@roo-code/types"
import { vscode } from "@src/utils/vscode"
import { useAppTranslation } from "@src/i18n/TranslationContext"
import { cn } from "@src/lib/utils"
import { prettyModelName } from "../../../utils/prettyModelName"
import { useProviderModels } from "../hooks/useProviderModels"
import { getModelIdKey, getSelectedModelId } from "../hooks/useSelectedModel"
import { usePreferredModels } from "@/components/ui/hooks/kilocode/usePreferredModels"
import { useGroupedModelIds } from "@/components/ui/hooks/kilocode/usePreferredModels"

interface ModelSelectorProps {
currentApiConfigName?: string
Expand All @@ -32,15 +32,66 @@ export const ModelSelector = ({
const modelIdKey = getModelIdKey({ provider })
const isAutocomplete = apiConfiguration.profileType === "autocomplete"

const modelsIds = usePreferredModels(providerModels)
const { preferredModelIds, restModelIds, hasPreferred } = useGroupedModelIds(providerModels)
const options = useMemo(() => {
const missingModelIds = modelsIds.indexOf(selectedModelId) >= 0 ? [] : [selectedModelId]
return missingModelIds.concat(modelsIds).map((modelId) => ({
value: modelId,
label: providerModels[modelId]?.displayName ?? prettyModelName(modelId),
type: DropdownOptionType.ITEM,
}))
}, [modelsIds, providerModels, selectedModelId])
const result: DropdownOption[] = []

// Check if selected model is missing from the lists
const allModelIds = [...preferredModelIds, ...restModelIds]
const isMissingSelectedModel = selectedModelId && !allModelIds.includes(selectedModelId)

// Add "Recommended models" section if there are preferred models
if (hasPreferred && preferredModelIds.length > 0) {
result.push({
value: "__label_recommended__",
label: t("settings:modelPicker.recommendedModels"),
type: DropdownOptionType.LABEL,
})

preferredModelIds.forEach((modelId) => {
result.push({
value: modelId,
label: providerModels[modelId]?.displayName ?? prettyModelName(modelId),
type: DropdownOptionType.ITEM,
})
})
}

// Add "All models" section
if (restModelIds.length > 0) {
result.push({
value: "__label_all__",
label: t("settings:modelPicker.allModels"),
type: DropdownOptionType.LABEL,
})

// Add missing selected model at the top of "All models" if not in any list
if (isMissingSelectedModel) {
result.push({
value: selectedModelId,
label: providerModels[selectedModelId]?.displayName ?? prettyModelName(selectedModelId),
type: DropdownOptionType.ITEM,
})
}

restModelIds.forEach((modelId) => {
result.push({
value: modelId,
label: providerModels[modelId]?.displayName ?? prettyModelName(modelId),
type: DropdownOptionType.ITEM,
})
})
} else if (isMissingSelectedModel) {
// If there are no rest models but we have a missing selected model, add it
result.push({
value: selectedModelId,
label: providerModels[selectedModelId]?.displayName ?? prettyModelName(selectedModelId),
type: DropdownOptionType.ITEM,
})
}

return result
}, [preferredModelIds, restModelIds, hasPreferred, providerModels, selectedModelId, t])

const disabled = isLoading || isError || isAutocomplete

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,11 @@ vi.mock("@/i18n/TranslationContext", () => ({
}),
}))

// Create a mock function that can be controlled per test
const mockUseGroupedModelIds = vi.fn()

vi.mock("@/components/ui/hooks/kilocode/usePreferredModels", () => ({
usePreferredModels: () => ["model-1", "model-2"],
useGroupedModelIds: () => mockUseGroupedModelIds(),
}))

// Create a mock function that can be controlled per test
Expand All @@ -37,9 +40,18 @@ describe("ModelSelector", () => {
}

beforeEach(() => {
// Reset mock before each test
// Reset mocks before each test
mockUseProviderModels.mockReset()
// Default mock implementation
mockUseGroupedModelIds.mockReset()

// Default mock implementation for useGroupedModelIds (no preferred models)
mockUseGroupedModelIds.mockReturnValue({
preferredModelIds: [],
restModelIds: ["model-1", "model-2"],
hasPreferred: false,
})

// Default mock implementation for useProviderModels
mockUseProviderModels.mockReturnValue({
provider: "openai",
providerModels: {
Expand Down Expand Up @@ -188,4 +200,101 @@ describe("ModelSelector", () => {
const dropdownTrigger = screen.queryByTestId("dropdown-trigger")
expect(dropdownTrigger).not.toBeInTheDocument()
})

describe("preferred models sections", () => {
test("builds options with section headers when preferred models exist", () => {
// Setup mock to return preferred models
mockUseGroupedModelIds.mockReturnValue({
preferredModelIds: ["preferred-1", "preferred-2"],
restModelIds: ["model-1", "model-2"],
hasPreferred: true,
})

mockUseProviderModels.mockReturnValue({
provider: "openai",
providerModels: {
"preferred-1": { displayName: "Preferred Model 1", preferredIndex: 0 },
"preferred-2": { displayName: "Preferred Model 2", preferredIndex: 1 },
"model-1": { displayName: "Model 1" },
"model-2": { displayName: "Model 2" },
},
providerDefaultModel: "model-1",
isLoading: false,
isError: false,
})

render(
<ModelSelector
currentApiConfigName="test-profile"
apiConfiguration={{
apiProvider: "openai",
apiModelId: "model-1",
}}
fallbackText="Select a model"
/>,
)

// Should render the dropdown
const dropdownTrigger = screen.getByTestId("dropdown-trigger")
expect(dropdownTrigger).toBeInTheDocument()
})

test("does not add section headers when no preferred models exist", () => {
// Setup mock with no preferred models
mockUseGroupedModelIds.mockReturnValue({
preferredModelIds: [],
restModelIds: ["model-1", "model-2"],
hasPreferred: false,
})

render(
<ModelSelector
currentApiConfigName="test-profile"
apiConfiguration={{
apiProvider: "openai",
apiModelId: "model-1",
}}
fallbackText="Select a model"
/>,
)

// Should render the dropdown
const dropdownTrigger = screen.getByTestId("dropdown-trigger")
expect(dropdownTrigger).toBeInTheDocument()
})

test("handles only preferred models without rest models", () => {
// Setup mock with only preferred models (edge case)
mockUseGroupedModelIds.mockReturnValue({
preferredModelIds: ["preferred-1"],
restModelIds: [],
hasPreferred: true,
})

mockUseProviderModels.mockReturnValue({
provider: "openai",
providerModels: {
"preferred-1": { displayName: "Preferred Model 1", preferredIndex: 0 },
},
providerDefaultModel: "preferred-1",
isLoading: false,
isError: false,
})

render(
<ModelSelector
currentApiConfigName="test-profile"
apiConfiguration={{
apiProvider: "openai",
apiModelId: "preferred-1",
}}
fallbackText="Select a model"
/>,
)

// Should render the dropdown with only preferred models section
const dropdownTrigger = screen.getByTestId("dropdown-trigger")
expect(dropdownTrigger).toBeInTheDocument()
})
})
})
85 changes: 50 additions & 35 deletions webview-ui/src/components/settings/ModelPicker.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useState, useCallback, useEffect, useRef, Fragment } from "react" // kilocode_change Fragment
import { useState, useCallback, useEffect, useRef, useMemo, Fragment } from "react" // kilocode_change Fragment, useMemo
import { VSCodeLink } from "@vscode/webview-ui-toolkit/react"
import { Trans } from "react-i18next"
import { ChevronsUpDown, Check, X, Info } from "lucide-react"
Expand All @@ -7,7 +7,7 @@ import type { ProviderSettings, ModelInfo, OrganizationAllowList } from "@roo-co

import { useAppTranslation } from "@src/i18n/TranslationContext"
import { useSelectedModel } from "@/components/ui/hooks/useSelectedModel"
import { usePreferredModels } from "@/components/ui/hooks/kilocode/usePreferredModels" // kilocode_change
import { useGroupedModelIds } from "@/components/ui/hooks/kilocode/usePreferredModels" // kilocode_change
// import { filterModels } from "./utils/organizationFilters" // kilocode_change: not doing this
import { cn } from "@src/lib/utils"
import {
Expand All @@ -21,7 +21,6 @@ import {
PopoverContent,
PopoverTrigger,
Button,
SelectSeparator, // kilocode_change
} from "@src/components/ui"
import { useEscapeKey } from "@src/hooks/useEscapeKey"

Expand Down Expand Up @@ -88,8 +87,9 @@ export const ModelPicker = ({
const selectTimeoutRef = useRef<NodeJS.Timeout | null>(null)
const closeTimeoutRef = useRef<NodeJS.Timeout | null>(null)

// kilocode_change start
const modelIds = usePreferredModels(models)
// kilocode_change start: Use grouped model IDs for section headers
const { preferredModelIds, restModelIds, hasPreferred } = useGroupedModelIds(models)
const modelIds = useMemo(() => [...preferredModelIds, ...restModelIds], [preferredModelIds, restModelIds])
const [isPricingExpanded, setIsPricingExpanded] = useState(false)
// kilocode_change end

Expand Down Expand Up @@ -205,36 +205,51 @@ export const ModelPicker = ({
</div>
)}
</CommandEmpty>
<CommandGroup>
{/* kilocode_change start */}
{modelIds.map((model, i) => {
const isPreferred = Number.isInteger(models?.[model]?.preferredIndex)
const previousModelWasPreferred = Number.isInteger(
models?.[modelIds[i - 1]]?.preferredIndex,
)
return (
<Fragment key={model}>
{!isPreferred && previousModelWasPreferred ? <SelectSeparator /> : null}
<CommandItem
value={model}
onSelect={onSelect}
data-testid={`model-option-${model}`}
className={cn(isPreferred ? "font-semibold" : "")}>
<span className="truncate" title={model}>
{model}
</span>
<Check
className={cn(
"size-4 p-0.5 ml-auto",
model === selectedModelId ? "opacity-100" : "opacity-0",
)}
/>
</CommandItem>
</Fragment>
)
})}
{/* kilocode_change end */}
</CommandGroup>
{/* kilocode_change start: Section headers for recommended and all models */}
{hasPreferred && preferredModelIds.length > 0 && (
<CommandGroup heading={t("settings:modelPicker.recommendedModels")}>
{preferredModelIds.map((model) => (
<CommandItem
key={model}
value={model}
onSelect={onSelect}
data-testid={`model-option-${model}`}
className="font-semibold">
<span className="truncate" title={model}>
{model}
</span>
<Check
className={cn(
"size-4 p-0.5 ml-auto",
model === selectedModelId ? "opacity-100" : "opacity-0",
)}
/>
</CommandItem>
))}
</CommandGroup>
)}
{restModelIds.length > 0 && (
<CommandGroup heading={t("settings:modelPicker.allModels")}>
{restModelIds.map((model) => (
<CommandItem
key={model}
value={model}
onSelect={onSelect}
data-testid={`model-option-${model}`}>
<span className="truncate" title={model}>
{model}
</span>
<Check
className={cn(
"size-4 p-0.5 ml-auto",
model === selectedModelId ? "opacity-100" : "opacity-0",
)}
/>
</CommandItem>
))}
</CommandGroup>
)}
{/* kilocode_change end */}
</CommandList>
{searchValue && !modelIds.includes(searchValue) && (
<div className="p-1 border-t border-vscode-input-border">
Expand Down
43 changes: 43 additions & 0 deletions webview-ui/src/components/ui/__tests__/select-dropdown.spec.tsx
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably just leave these tests out– they aren't checking anything useful

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I think I removed the section you were referring to.

Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,49 @@ describe("SelectDropdown", () => {
expect(content).toBeInTheDocument()
})

// kilocode_change start: Tests for LABEL type (section headers)
it("renders label options as section headers", () => {
const optionsWithLabel = [
{ value: "__label_recommended__", label: "Recommended models", type: DropdownOptionType.LABEL },
{ value: "model1", label: "Model 1" },
{ value: "__label_all__", label: "All models", type: DropdownOptionType.LABEL },
{ value: "model2", label: "Model 2" },
]

render(<SelectDropdown value="model1" options={optionsWithLabel} onChange={onChangeMock} />)

// Click the trigger to open the dropdown
const trigger = screen.getByTestId("dropdown-trigger")
fireEvent.click(trigger)

// Now we can check for dropdown content
const content = screen.getByTestId("dropdown-content")
expect(content).toBeInTheDocument()
})

it("LABEL options are non-selectable", () => {
const optionsWithLabel = [
{ value: "__label_section__", label: "Section Header", type: DropdownOptionType.LABEL },
{ value: "option1", label: "Option 1" },
]

render(<SelectDropdown value="option1" options={optionsWithLabel} onChange={onChangeMock} />)

// Click the trigger to open the dropdown
const trigger = screen.getByTestId("dropdown-trigger")
fireEvent.click(trigger)

// The content should be rendered
const content = screen.getByTestId("dropdown-content")
expect(content).toBeInTheDocument()

// Note: The actual component renders LABEL items as static divs without onClick handlers
// (with data-testid="dropdown-label"), making them inherently non-selectable.
// This test uses mocks, so we just verify the component renders correctly.
// The non-selectability behavior is ensured by the component's implementation.
})
// kilocode_change end

it("calls onChange for regular menu items", () => {
render(<SelectDropdown value="option1" options={options} onChange={onChangeMock} />)

Expand Down
Loading