-
Notifications
You must be signed in to change notification settings - Fork 18
Implement all models #303
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Implement all models #303
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR integrates kilocode models into the system by updating both the API and UI layers, replacing hardcoded mappings with dynamic, API-fetched model details.
- Updates selection logic for kilocode models in the UI
- Refactors API handlers to use KilocodeOpenrouterHandler while removing the deprecated KiloCodeHandler
- Adjusts schema and type definitions to support flexible kilocode model values
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
webview-ui/src/components/ui/hooks/useSelectedModel.ts | Updated kilocode model selection and fallback logic |
webview-ui/src/components/settings/ModelPicker.tsx | Extended model picker to support kilocode models and improved sorting |
webview-ui/src/components/settings/ApiOptions.tsx | Replaced hardcoded kilocode descriptions with dynamic model picker usage |
src/shared/api.ts | Added "kilocode-openrouter" to router names |
src/schemas/index.ts | Modified modelInfoSchema and providerSettingsSchema for kilocode flexibility |
src/exports/types.ts | Updated kilocodeModel type to a string |
src/core/webview/webviewMessageHandler.ts | Included kilocode model fetch in the router model handling |
src/core/Cline.ts | Imports and utilizes KilocodeOpenrouterHandler for fetching model info |
src/api/providers/kilocode.ts | Removed obsolete KiloCodeHandler |
src/api/providers/kilocode-openrouter.ts | Refactored getModel and added fetchModel with sorting logic for kilocode models |
src/api/providers/fetchers/openrouter.ts | Added support for optional headers in API requests |
src/api/providers/fetchers/cache.ts | Extended getModels to fetch kilocode models with authorization |
src/api/index.ts | Updated API handler builder to use KilocodeOpenrouterHandler for kilocode |
gpt41: "openai/gpt-4.1", | ||
gemini25flashpreview: "google/gemini-2.5-flash-preview", | ||
claude37: "anthropic/claude-3.7-sonnet", | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an easy way to set the selected model in the settings to the new model id's automatically when encountering an old model name? In that way we could eventually remove this legacy mapping once new updates have been released long enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
like a "migration" ?
|
||
// First add preferred models, sorted by preferredIndex | ||
Object.entries(models) | ||
.filter(([_, model]) => model.preferredIndex !== undefined) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why filter out models first without a preferredIndex only to do another loop for them to be added at the end. Lets handle this in the sort function so the double Object.entries loop is not necessary
}) | ||
.forEach(([id, model]) => { | ||
sortedModels[id] = { ...model } | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would prefer for this to be a map function that's just returned instead of first initiating a const of which the object is then basically modified.
|
||
function getKiloBaseUri(options: ApiHandlerOptions) { | ||
try { | ||
const token = options.kilocodeToken as string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as string
is unnecessary, already handled in the ApiHandlerOptions
type
"Gemini 2.5 Flash Preview is Google's most capable model for reasoning, coding, and multimodal tasks.", | ||
} | ||
// kilocode_change end | ||
// kilocode_change removed unused descriptions as they're now fetched from the API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this comment is unnecessary. It's back to original now so there's no change anymore :)
restModelIds.push(key) | ||
} | ||
} | ||
restModelIds.sort((a, b) => a.localeCompare(b)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels unnecessary since the models are already sorted like this in the kilocode-openrouter provider, We should only sort this either here or in the provider, no need to do in 2 places.
|
||
if (sortedModels.length > 0) { | ||
return sortedModels[0][1] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels like another double sorting?
Context
Allow the use of all Openrouter models through our proxy.
Screencast.from.09-05-25.11.30.45.webm