-
Notifications
You must be signed in to change notification settings - Fork 3.5k
fix(providers): Ollama default comes from the live local catalog, not a hosted costume #5795
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 |
|---|---|---|
|
|
@@ -688,7 +688,7 @@ | |
| "label": "Ollama", | ||
| "endpoint": "http://localhost:11434/v1", | ||
| "wire": "chat-completions", | ||
| "defaultModel": "deepseek-v4-flash", | ||
| "defaultModel": "unknown", | ||
|
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. [WARNING] Public providers export exposes the internal
|
||
| "envVars": [ | ||
| "OLLAMA_API_KEY" | ||
| ], | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -137,7 +137,16 @@ pub const DEFAULT_SGLANG_BASE_URL: &str = "http://localhost:30000/v1"; | |
| pub const DEFAULT_VLLM_MODEL: &str = "deepseek-ai/DeepSeek-V4-Pro"; | ||
| pub const DEFAULT_VLLM_FLASH_MODEL: &str = "deepseek-ai/DeepSeek-V4-Flash"; | ||
| pub const DEFAULT_VLLM_BASE_URL: &str = "http://localhost:8000/v1"; | ||
| pub const DEFAULT_OLLAMA_MODEL: &str = "deepseek-v4-flash"; | ||
| /// Unresolved local-Ollama default. A live provider-catalog refresh must | ||
| /// replace this with a tag `GET /v1/models` actually returned. Do not send | ||
| /// this string as a model id. | ||
| pub const DEFAULT_OLLAMA_MODEL: &str = "unknown"; | ||
|
|
||
|
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. [INFO] Because |
||
| /// True when `model` is the pre-refresh local-Ollama placeholder, not a tag. | ||
| #[must_use] | ||
| pub fn is_unresolved_local_ollama_model(model: &str) -> bool { | ||
| model.trim().eq_ignore_ascii_case(DEFAULT_OLLAMA_MODEL) | ||
|
Comment on lines
+140
to
+148
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. [INFO] A real local Ollama tag named 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. Add a unit test for |
||
| } | ||
| pub const DEFAULT_OLLAMA_BASE_URL: &str = "http://localhost:11434/v1"; | ||
| pub const DEFAULT_OLLAMA_CLOUD_MODEL: &str = "gpt-oss:120b"; | ||
| pub const DEFAULT_OLLAMA_CLOUD_BASE_URL: &str = codewhale_config::provider::OLLAMA_CLOUD_BASE_URL; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -366,6 +366,30 @@ fn configured_model_for_provider(config: &Config, provider: ApiProvider) -> Opti | |
| } | ||
|
|
||
| fn provider_default_model(config: &Config, provider: ApiProvider) -> String { | ||
| if provider == ApiProvider::Ollama { | ||
| let configured = if provider == config.api_provider() { | ||
| Some(config.default_model()) | ||
| } else { | ||
| configured_model_for_provider(config, provider) | ||
| }; | ||
| let unresolved = configured.as_deref().is_none_or(|model| { | ||
| model.trim().eq_ignore_ascii_case("auto") | ||
| || crate::config::is_unresolved_local_ollama_model(model) | ||
| }); | ||
|
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. [INFO] First live Ollama tag is used without considering
|
||
| if unresolved | ||
| && let Some(live) = crate::provider_lake::live_per_provider_models(provider) | ||
|
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. [WARNING]
|
||
| .into_iter() | ||
| .next() | ||
| { | ||
| return live; | ||
| } | ||
|
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. Filter live offerings to the chat endpoint and prefer the offering flagged |
||
| if let Some(model) = configured.filter(|model| { | ||
| !model.trim().eq_ignore_ascii_case("auto") | ||
| && !crate::config::is_unresolved_local_ollama_model(model) | ||
| }) { | ||
| return model; | ||
|
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. [WARNING] Unresolved Ollama default can still fall through to older resolution paths The new Ollama branch in 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. After the configured-model filter block inside the Ollama branch, return the unresolved default immediately so unresolved Ollama never falls through to generic resolution that could consult Models.dev or the bundled catalog. |
||
| } | ||
| } | ||
| if provider == config.api_provider() { | ||
| let model = config.default_model(); | ||
| if !model.trim().eq_ignore_ascii_case("auto") { | ||
|
|
@@ -1107,4 +1131,33 @@ mod tests { | |
| }; | ||
| assert!(!ModelInventory::from_config(&deepseek).router_available); | ||
| } | ||
|
|
||
| #[test] | ||
| fn ollama_default_prefers_live_local_tags_over_the_unresolved_marker() { | ||
| let _live = crate::provider_lake::lock_live_snapshot(); | ||
| crate::provider_lake::clear_live_snapshot(); | ||
| let config = Config { | ||
| provider: Some("ollama".to_string()), | ||
| ..Default::default() | ||
| }; | ||
| assert_eq!( | ||
| provider_default_model(&config, ApiProvider::Ollama), | ||
| crate::config::DEFAULT_OLLAMA_MODEL | ||
| ); | ||
|
|
||
| crate::provider_lake::merge_live_offerings(vec![ | ||
| codewhale_config::catalog::CatalogOffering { | ||
| provider: "ollama".to_string(), | ||
| wire_model_id: "qwen2.5:0.5b".to_string(), | ||
| endpoint_key: "chat".to_string(), | ||
| default_for_provider: true, | ||
| ..Default::default() | ||
| }, | ||
| ]); | ||
| assert_eq!( | ||
| provider_default_model(&config, ApiProvider::Ollama), | ||
| "qwen2.5:0.5b" | ||
| ); | ||
| crate::provider_lake::clear_live_snapshot(); | ||
| } | ||
| } | ||
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.
[WARNING] Ollama request path can still submit the
unknownplaceholder before the live refresh arrivesChanging
DEFAULT_OLLAMA_MODELtounknownmeans any code path that callsconfig.default_model()directly will send an invalid model id before the background refresh has populated live tags. The newis_unresolved_local_ollama_modelhelper is only used inprovider_default_model; no request/dispatch guard appears in this diff. A user who submits immediately, or whose local Ollama daemon is unavailable, will getmodel 'unknown' not foundrather than the previous hosted-model failure.