-
Notifications
You must be signed in to change notification settings - Fork 6.3k
feat(opencode): add auto model detection for OpenAI-compatible providers #8359
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: dev
Are you sure you want to change the base?
Conversation
|
The following comment was made by an LLM, it may be inaccurate: Potential Duplicate PRs FoundBased on the search results, there are two PRs that appear to be related to or potentially addressing similar functionality:
Recommendation: You should review these PRs to check if they have been merged, closed, or superseded, and whether PR #8359 builds upon or duplicates their work. |
|
I was going to add this this week, some notes:
|
|
|
I wanted to add this as well! LMStudio does have api to fetch each model info with context length etc |
It's not like "completely dropped" it should be like any model that exists in models.dev that isnt in models endpoint should be dropped but that doesnt mean drop all the useful metadata it's more to filter potentially unsupported models |
Then we should defo use that, wanna do the same for ollama but that can be separate |
|
@rekram1-node I would love to help out on this if possible |
|
Yeah up to yall I'd really like to ship this week so @tomzhu1024 if you aren't going to be available I think @goniz or myself will take care of this. Ig we can build off this PR easily |
Got it. This is definitely the optimal way. Thanks for the clarification! I was planning to iterate this PR later today -- I'm planning to 1) remove the extra config option, 2) still honor the model info from Models.dev (if they do exist according to the endpoint), 3) leave some space for provider-specific model info retrieval (like @goniz mentioned). I'm a little unsure about expanding this feature to all But feel free to take it over; just let me know so I can shift to something else. |
All the providers should work, I can test and iterate to make sure it works but ik u would use your github token from oauth exchange for copilot, and an api key for zen, etc so i think most things will be pretty standard. |
|
I see. That sounds good. I'll then try to 4) expand it to all |
|
I've got initial draft for probing loaded models for LMStudio and Ollama. I'm using the OllamaLMStudioThe output is raw atm but I'll fix it tomorrow then I'll push something |
23396df to
5080e14
Compare
|
I just got a chance to work on it. Now:
|
5080e14 to
875c8bc
Compare
| } | ||
|
|
||
| // auto-detect models | ||
| for (const [providerID, provider] of Object.entries(providers)) { |
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.
We need to make this somewhat optimal so make sure:
- this isnt running for every single provider in the entire models list (only authed ones) << or maybe we need to move this logic to behave differently?
- we should Promise.all or use worker queue to prevent super slow linear requests
- we should have timeouts (looks like u added that already -- I think we should go lower tho like 1-3 sec tops at least if it is gonna be blocking...)
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.
- Do you mean this should run only for providers that are "activated" via the config or auth json or env var, rather than for all providers listed in Models.dev? If so, it already works that way.
- As for the placement, I put it near the end because that’s where enabled providers are picked and their auth information is collected. Do you see a better place for it?
- Sure, I'll use
Promise.all. - I'll reduce the timeout to 3 secs. Since all providers are queried in parallel, this shouldn’t add much lag.
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.
Done. I moved this slightly in provider.ts so custom providers like "ollama" aren’t skipped just because they don’t exist in Models.dev, and the model.api.npm patch for @ai-sdk/github-copilot no longer has to run twice.
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.
awesome, ill take a look
|
I've pushed my PR with the ollama+lmstudio apis. |
|
/review |
| const providerNPM = configProvider?.npm ?? modelsDevProvider?.npm ?? "@ai-sdk/openai-compatible" | ||
| const providerBaseURL = configProvider?.options?.baseURL ?? configProvider?.api ?? modelsDevProvider?.api | ||
|
|
||
| let detectedModels: Record<string, Partial<Provider.Model>> | undefined = 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.
Style suggestion: Consider refactoring to avoid the let statement. You could use an early return pattern or the Result pattern. For example:
if (providerNPM !== "@ai-sdk/openai-compatible" || !providerBaseURL) return
const detectedModels = await ProviderModelDetection.OpenAICompatible.listModels(providerBaseURL, provider)This eliminates the need for let and the subsequent if (detectedModels === undefined) return check becomes unnecessary.
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 is mainly for future extensibility, like we may have model detection for non-@ai-sdk/openai-compatible providers, or we may have a different detection method for a specific @ai-sdk/compatible provider (e.g. LMStudio/Ollama).
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.
Fixed.
| type OpenAICompatibleResponse = z.infer<typeof OpenAICompatibleResponse> | ||
|
|
||
| export async function listModels(baseURL: string, provider: Provider.Info): Promise<Record<string, Partial<Provider.Model>>> { | ||
| let res: Response |
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.
Style suggestion: These let statements combined with multiple try/catch blocks could potentially be refactored. The style guide advises avoiding let and try/catch where possible.
One option is to let exceptions propagate and handle them at the call site (which is already wrapped in a try/catch in provider.ts). This would simplify the function to:
const res = await fetchFn(`${baseURL}/models`, {
headers,
signal: AbortSignal.timeout(3 * 1000),
})
if (!res.ok) throw new Error(`bad http status ${res.status}`)
const parsed = OpenAICompatibleResponse.parse(await res.json())The error messages would be less descriptive, but the call site already logs the error. Up to you whether the explicit error wrapping is worth the complexity.
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.
Fixed.
What does this PR do?
This PR adds automatic model detection/discovery for providers that use
npm: @ai-sdk/openai-compatible. It allows OpenCode to utilize theGET /v1/modelsAPI to populate the model list.Models may come from 3 sources now: Models.dev, config file, and detected via API. The "filtering" logic is: the model will be visible, if it's detected via API or declared in config file; if it is visible, its metadata will be merged from API -> config -> Models.dev (with prescedence from high to low).
Fixes #6231
Fixes #4232
Fixes #2456
Note: I understand it may be by design to rely entirely on
Models.devfor providers and models, but having OpenCode core automatically populate the model list would be a major improvement for users running LM Studio or other local providers.How did you verify your code works?
openai/gpt-oss-20bandzai-org/glm-4.6v-flash).~/.config/opencode/opencode.jsonwith two LM Studio providers and two models. One of the models does not exist in LM Studio, but should still appear later. (File attached: opencode.json).providersvalue atpackages/opencode/src/provider/provider.ts#958, and it looked correct to me.bun dev -- models, and the output looked correct.bun dev, ran/models, and the output also looked correct.