vrr/zed
2
0
Fork 0
mirror of https://github.com/zed-industries/zed.git synced 2026-07-09 16:00:35 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 16

ollama: Don't abort entire model fetch if one model's details fail to load (#59606)

Browse source 
## Summary

Fixes #37815.

`State::fetch_models()` calls `/api/tags` to list models, then calls
`/api/show` for **every** model in that list to get its capabilities,
collecting the results with `collect::<Result<Vec<_>>>()?`. If
`/api/show` errors for even one model, the whole batch fails and
`fetched_models` is never populated. Since `is_authenticated()` is
defined as `!self.fetched_models.is_empty()`, this means a single bad
model permanently breaks both authentication state and the model picker
for the entire Ollama provider - with no error surfaced anywhere (not
the UI, not the logs), which matches the reports in #37815 of "Connect
does nothing" / "no logs, no nothing".

I hit this myself: I had a stale local reference to an Ollama Cloud
model that had been retired server-side. `/api/show` for that one model
returned `410 Gone`, which silently broke Connect and the model picker
for every other model too. Removing the retired model with `ollama rm`
fixed it immediately, which confirmed the root cause.

## Fix

Instead of aborting the whole fetch on the first error, skip individual
models that fail `/api/show` and log a warning, keeping the rest.
Extracted this into a small `skip_failed_models` helper so it's
unit-testable without mocking HTTP.

## Disclosure

I used Claude (Anthropic's Claude Code) to help track down this root
cause (tracing through `fetch_models`/`is_authenticated` in this file)
and draft the fix + tests below. I reviewed and understand the change -
it's a small, targeted fix to a single function plus two unit tests for
the new helper.

## Test plan

- [x] `cargo check -p language_models` passes
- [x] `cargo test -p language_models --lib ollama::` - all 3 tests pass
(the 2 new ones plus the existing
`test_merge_settings_preserves_display_names_for_similar_models`,
unaffected by this change)
- [x] `cargo fmt -p language_models -- --check` - no diff

## Release Notes

Release Notes:

- Fixed Ollama models silently failing to show up in the model picker
(and "Connect" appearing to do nothing) when a single model's details
couldn't be fetched, e.g. a retired Ollama Cloud model

---------

Co-authored-by: MrSubidubi <finn@zed.dev>
Co-authored-by: Ben Brandt <benjamin.j.brandt@gmail.com>
This commit is contained in:
Manoj Mishra 2026-06-23 14:51:31 +05:30 • committed by GitHub
parent cf76418cf4
commit 4eb039b451
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 135 additions and 46 deletions
Download patch file Download diff file Expand all files Collapse all files

4
crates/acp_thread/src/connection.rs
View file

@ -4,7 +4,7 @@ use anyhow::Result;
use chrono::{DateTime, Utc};
use collections::{HashMap, HashSet, IndexMap};
use gpui::{Entity, SharedString, Task};
use language_model::LanguageModelProviderId;
use language_model::{DisabledReason, LanguageModelProviderId};
use project::{AgentId, Project};
use serde::{Deserialize, Serialize};
use std::{any::Any, error::Error, fmt, path::PathBuf, rc::Rc};
@ -493,6 +493,7 @@ pub struct AgentModelInfo {
pub icon: Option<AgentModelIcon>,
pub is_latest: bool,
pub cost: Option<SharedString>,
pub disabled: Option<DisabledReason>,
}
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
@ -1064,6 +1065,7 @@ mod test_support {
icon: Some(AgentModelIcon::Named(ui::IconName::ZedAssistant)),
is_latest: false,
cost: None,
disabled: None,
})),
}
}

2
crates/agent/src/agent.rs
View file

@ -310,6 +310,7 @@ impl LanguageModels {
}),
is_latest: model.is_latest(),
cost: model.model_cost_info().map(|cost| cost.to_shared_string()),
disabled: model.is_disabled(),
}
}
@ -5545,6 +5546,7 @@ mod internal_tests {
ui::IconName::ZedAssistant
)),
is_latest: false,
disabled: None,
cost: None,
}]
)])

9
crates/agent_ui/src/model_selector.rs
View file

@ -271,6 +271,7 @@ impl PickerDelegate for ModelPickerDelegate {
fn confirm(&mut self, _secondary: bool, window: &mut Window, cx: &mut Context<Picker<Self>>) {
if let Some(ModelPickerEntry::Model(model_info, _)) =
self.filtered_entries.get(self.selected_index)
&& model_info.disabled.is_none()
{
self.selector
.select_model(model_info.id.clone(), cx)
@ -336,6 +337,7 @@ impl PickerDelegate for ModelPickerDelegate {
Some(AgentModelIcon::Named(icon)) => this.icon(*icon),
None => this,
})
.disabled(model_info.disabled.clone())
.is_selected(is_selected)
.is_focused(selected)
.is_latest(model_info.is_latest)
@ -511,6 +513,7 @@ mod tests {
description: None,
icon: None,
is_latest: false,
disabled: None,
cost: None,
})
.collect::<Vec<_>>(),
@ -618,6 +621,7 @@ mod tests {
description: None,
icon: None,
is_latest: false,
disabled: None,
cost: None,
},
AgentModelInfo {
@ -626,6 +630,7 @@ mod tests {
description: None,
icon: None,
is_latest: false,
disabled: None,
cost: None,
},
];
@ -810,6 +815,7 @@ mod tests {
description: None,
icon: None,
is_latest: false,
disabled: None,
cost: None,
},
acp_thread::AgentModelInfo {
@ -818,6 +824,7 @@ mod tests {
description: None,
icon: None,
is_latest: false,
disabled: None,
cost: None,
},
]);
@ -860,6 +867,7 @@ mod tests {
description: None,
icon: None,
is_latest: false,
disabled: None,
cost: None,
},
acp_thread::AgentModelInfo {
@ -868,6 +876,7 @@ mod tests {
description: None,
icon: None,
is_latest: false,
disabled: None,
cost: None,
},
]);

75
crates/agent_ui/src/ui/model_selector_components.rs
View file

@ -1,4 +1,5 @@
use gpui::{Action, ClickEvent, FocusHandle, prelude::*};
use language_model::DisabledReason;
use ui::{Chip, ElevationIndex, KeyBinding, ListItem, ListItemSpacing, Tooltip, prelude::*};
use zed_actions::agent::ToggleModelSelector;
@ -52,6 +53,7 @@ pub struct ModelSelectorListItem {
is_focused: bool,
is_latest: bool,
is_favorite: bool,
disabled: Option<DisabledReason>,
on_toggle_favorite: Option<Box<dyn Fn(&ClickEvent, &mut Window, &mut App) + 'static>>,
cost_info: Option<SharedString>,
}
@ -66,6 +68,7 @@ impl ModelSelectorListItem {
is_focused: false,
is_latest: false,
is_favorite: false,
disabled: None,
on_toggle_favorite: None,
cost_info: None,
}
@ -86,6 +89,11 @@ impl ModelSelectorListItem {
self
}
pub fn disabled(mut self, disabled: Option<DisabledReason>) -> Self {
self.disabled = disabled;
self
}
pub fn is_focused(mut self, is_focused: bool) -> Self {
self.is_focused = is_focused;
self
@ -117,8 +125,12 @@ impl ModelSelectorListItem {
impl RenderOnce for ModelSelectorListItem {
fn render(self, _window: &mut Window, _cx: &mut App) -> impl IntoElement {
let is_disabled = self.disabled.is_some();
let model_icon_color = if self.is_selected {
Color::Accent
} else if is_disabled {
Color::Disabled
} else {
Color::Muted
};
@ -129,6 +141,15 @@ impl RenderOnce for ModelSelectorListItem {
.inset(true)
.spacing(ListItemSpacing::Sparse)
.toggle_state(self.is_focused)
.when_some(self.disabled, |this, disabled_reason| {
this.disabled(true)
.tooltip(Tooltip::text(disabled_reason.0))
.end_slot(
div()
.pr_2()
.child(Icon::new(IconName::Info).color(Color::Muted)),
)
})
.child(
h_flex()
.w_full()
@ -143,7 +164,11 @@ impl RenderOnce for ModelSelectorListItem {
.size(IconSize::Small),
)
})
.child(Label::new(self.title).truncate())
.child(
Label::new(self.title)
.when(is_disabled, |this| this.color(Color::Disabled))
.truncate(),
)
.when(self.is_latest, |parent| parent.child(Chip::new("Latest")))
.when_some(self.cost_info, |this, cost_info| {
let tooltip_text = if cost_info.ends_with('×') {
@ -157,26 +182,34 @@ impl RenderOnce for ModelSelectorListItem {
this.child(Chip::new(cost_info).tooltip(Tooltip::text(tooltip_text)))
}),
)
.end_slot(div().pr_2().when(self.is_selected, |this| {
this.child(Icon::new(IconName::Check).color(Color::Accent))
}))
.end_slot_on_hover(div().pr_1p5().when_some(self.on_toggle_favorite, {
|this, handle_click| {
let (icon, color, tooltip) = if is_favorite {
(IconName::StarFilled, Color::Accent, "Unfavorite Model")
} else {
(IconName::Star, Color::Default, "Favorite Model")
};
this.child(
IconButton::new(("toggle-favorite", self.index), icon)
.layer(ElevationIndex::ElevatedSurface)
.icon_color(color)
.icon_size(IconSize::Small)
.tooltip(Tooltip::text(tooltip))
.on_click(move |event, window, cx| (handle_click)(event, window, cx)),
)
}
}))
.when(self.is_selected, |this| {
this.end_slot(
div()
.pr_2()
.child(Icon::new(IconName::Check).color(Color::Accent)),
)
})
.when(!is_disabled, |this| {
this.end_slot_on_hover(div().pr_1p5().when_some(self.on_toggle_favorite, {
|this, handle_click| {
let (icon, color, tooltip) = if is_favorite {
(IconName::StarFilled, Color::Accent, "Unfavorite Model")
} else {
(IconName::Star, Color::Default, "Favorite Model")
};
this.child(
IconButton::new(("toggle-favorite", self.index), icon)
.layer(ElevationIndex::ElevatedSurface)
.icon_color(color)
.icon_size(IconSize::Small)
.tooltip(Tooltip::text(tooltip))
.on_click(move |event, window, cx| {
(handle_click)(event, window, cx)
}),
)
}
}))
})
}
}

14
crates/language_model/src/language_model.rs
View file

@ -24,6 +24,15 @@ pub fn init(cx: &mut App) {
registry::init(cx);
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct DisabledReason(pub SharedString);
impl DisabledReason {
pub fn new(reason: impl Into<SharedString>) -> Self {
Self(reason.into())
}
}
pub struct LanguageModelTextStream {
pub message_id: Option<String>,
pub stream: BoxStream<'static, Result<String, LanguageModelCompletionError>>,
@ -58,6 +67,11 @@ pub trait LanguageModel: Send + Sync {
false
}
/// Whether the model is currently disabled and, if so, why this is the case.
fn is_disabled(&self) -> Option<DisabledReason> {
None
}
/// Whether requests to this model require the user to consent to the
/// upstream provider retaining inference logs (i.e. the model cannot be
/// offered with Zero Data Retention).

57
crates/language_models/src/provider/ollama.rs
View file

@ -7,11 +7,12 @@ use futures::{Stream, TryFutureExt, stream};
use gpui::{AnyView, App, AsyncApp, Context, CursorStyle, Entity, Task, TaskExt};
use http_client::{CustomHeaders, HttpClient};
use language_model::{
ApiKeyState, AuthenticateError, EnvVar, IconOrSvg, LanguageModel, LanguageModelCompletionError,
LanguageModelCompletionEvent, LanguageModelId, LanguageModelName, LanguageModelProvider,
LanguageModelProviderId, LanguageModelProviderName, LanguageModelProviderState,
LanguageModelRequest, LanguageModelRequestTool, LanguageModelToolChoice, LanguageModelToolUse,
LanguageModelToolUseId, MessageContent, RateLimiter, Role, StopReason, TokenUsage, env_var,
ApiKeyState, AuthenticateError, DisabledReason, EnvVar, IconOrSvg, LanguageModel,
LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelId, LanguageModelName,
LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName,
LanguageModelProviderState, LanguageModelRequest, LanguageModelRequestTool,
LanguageModelToolChoice, LanguageModelToolUse, LanguageModelToolUseId, MessageContent,
RateLimiter, Role, StopReason, TokenUsage, env_var,
};
use menu;
use ollama::{
@ -139,23 +140,32 @@ impl State {
let extra_headers = extra_headers.clone();
async move {
let name = model.name.as_str();
let model = show_model(
show_model(
http_client.as_ref(),
&api_url,
api_key.as_deref(),
name,
&extra_headers,
)
.await?;
let ollama_model = ollama::Model::new(
name,
None,
model.context_length,
Some(model.supports_tools()),
Some(model.supports_vision()),
Some(model.supports_thinking()),
);
Ok(ollama_model)
.await
.map_or_else(
|error| {
ollama::Model::new_disabled(
name,
format!("Failed to fetch model from API: {error}",),
)
},
|model| {
ollama::Model::new(
name,
model.context_length,
Some(model.supports_tools()),
Some(model.supports_vision()),
Some(model.supports_thinking()),
)
},
)
}
});
@ -163,10 +173,8 @@ impl State {
// since there is an arbitrary number of models available
let mut ollama_models: Vec<_> = futures::stream::iter(tasks)
.buffer_unordered(5)
.collect::<Vec<Result<_>>>()
.await
.into_iter()
.collect::<Result<Vec<_>>>()?;
.collect()
.await;
ollama_models.sort_by(|a, b| a.name.cmp(&b.name));
@ -308,6 +316,7 @@ impl LanguageModelProvider for OllamaLanguageModelProvider {
.map(|model| {
Arc::new(OllamaLanguageModel {
id: LanguageModelId::from(model.name.clone()),
disabled: model.disabled.as_ref().map(|d| DisabledReason::new(d)),
model,
http_client: self.http_client.clone(),
request_limiter: RateLimiter::new(4),
@ -350,6 +359,7 @@ pub struct OllamaLanguageModel {
http_client: Arc<dyn HttpClient>,
request_limiter: RateLimiter,
state: Entity<State>,
disabled: Option<DisabledReason>,
}
impl OllamaLanguageModel {
@ -510,6 +520,10 @@ impl LanguageModel for OllamaLanguageModel {
format!("ollama/{}", self.model.id())
}
fn is_disabled(&self) -> Option<DisabledReason> {
self.disabled.clone()
}
fn max_token_count(&self) -> u64 {
self.model.max_token_count()
}
@ -1089,6 +1103,7 @@ fn merge_settings_into_models(
supports_tools: setting_model.supports_tools,
supports_vision: setting_model.supports_images,
supports_thinking: setting_model.supports_thinking,
disabled: None,
},
);
}
@ -1126,6 +1141,7 @@ mod tests {
supports_tools: None,
supports_vision: None,
supports_thinking: None,
disabled: None,
},
);
models.insert(
@ -1138,6 +1154,7 @@ mod tests {
supports_tools: None,
supports_vision: None,
supports_thinking: None,
disabled: None,
},
);

20
crates/ollama/src/ollama.rs
View file

@ -21,6 +21,7 @@ pub struct Model {
pub supports_tools: Option<bool>,
pub supports_vision: Option<bool>,
pub supports_thinking: Option<bool>,
pub disabled: Option<String>,
}
fn get_max_tokens(_name: &str) -> u64 {
@ -31,7 +32,6 @@ fn get_max_tokens(_name: &str) -> u64 {
impl Model {
pub fn new(
name: &str,
display_name: Option<&str>,
max_tokens: Option<u64>,
supports_tools: Option<bool>,
supports_vision: Option<bool>,
@ -39,14 +39,26 @@ impl Model {
) -> Self {
Self {
name: name.to_owned(),
display_name: display_name
.map(ToString::to_string)
.or_else(|| name.strip_suffix(":latest").map(ToString::to_string)),
display_name: name.strip_suffix(":latest").map(ToString::to_string),
max_tokens: max_tokens.unwrap_or_else(|| get_max_tokens(name)),
keep_alive: Some(KeepAlive::indefinite()),
supports_tools,
supports_vision,
supports_thinking,
disabled: None,
}
}
pub fn new_disabled(name: &str, reason: String) -> Self {
Self {
name: name.to_owned(),
display_name: name.strip_suffix(":latest").map(ToString::to_string),
max_tokens: get_max_tokens(name),
keep_alive: Some(KeepAlive::indefinite()),
supports_tools: None,
supports_vision: None,
supports_thinking: None,
disabled: Some(reason),
}
}