Repository navigation
fix(api): honor custom model IDs for OpenAI-compatible providers #1846
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| import { Anthropic } from "@anthropic-ai/sdk" | ||
| import OpenAI from "openai" | ||
|
|
||
| import type { ModelInfo } from "@roo-code/types" | ||
| import { type ModelInfo, openAiModelInfoSaneDefaults } from "@roo-code/types" | ||
|
|
||
| import { type ApiHandlerOptions, getModelMaxOutputTokens } from "../../shared/api" | ||
| import { TagMatcher } from "../../utils/tag-matcher" | ||
|
|
@@ -243,11 +243,27 @@ export abstract class BaseOpenAiCompatibleProvider<ModelName extends string> | |
| } | ||
|
|
||
| override getModel() { | ||
| const id = | ||
| this.options.apiModelId && this.options.apiModelId in this.providerModels | ||
| ? (this.options.apiModelId as ModelName) | ||
| : this.defaultProviderModelId | ||
| const requestedId = this.options.apiModelId | ||
|
|
||
| return { id, info: this.providerModels[id] } | ||
| // A known model: use its predefined metadata. | ||
| if (requestedId && requestedId in this.providerModels) { | ||
| const id = requestedId as ModelName | ||
| return { id, info: this.providerModels[id] } | ||
| } | ||
|
|
||
| // A user-supplied custom model that isn't in our static list (e.g. a newly | ||
| // released Fireworks model). Honor the exact id the user configured instead | ||
| // of silently falling back to the provider default, which would send the | ||
| // wrong model to the API and can surface as a confusing "model not found" | ||
| // error. Provide sane default metadata so the rest of the pipeline works. | ||
| if (requestedId) { | ||
| return { | ||
|
p12tic marked this conversation as resolved.
|
||
| id: requestedId as ModelName, | ||
| info: { ...openAiModelInfoSaneDefaults }, | ||
|
p12tic marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in #1847, max tokens is removed altogether there. |
||
| } | ||
| } | ||
|
|
||
| // No model configured: use the provider default. | ||
| return { id: this.defaultProviderModelId, info: this.providerModels[this.defaultProviderModelId] } | ||
| } | ||
| } | ||
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 only asserts
model:. Could we also assert themax_tokensin the request for a custom id (undefined or > 0)? That would cover the sane-defaults to request path this PR adds.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.
Addressed in #1847, max tokens is removed altogether there.