From cf6ae01d07b5fd02629535250ebddc65f9d0d9ed Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 11 Nov 2025 17:10:46 -0500 Subject: [PATCH] Show recommended models under normal category too (#42489) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Screenshot 2025-11-11 at 4 04 57 PM Discussed with @danilo-leal and we're going with the "it's checked in both places" design! Closes #40910 Release Notes: - Recommended AI models now still appear in their normal category in addition to "Recommended:" --- crates/agent/src/agent.rs | 4 +- .../agent_ui/src/language_model_selector.rs | 82 ++++++++----------- 2 files changed, 34 insertions(+), 52 deletions(-) diff --git a/crates/agent/src/agent.rs b/crates/agent/src/agent.rs index fc0b66f4073ea137f53b29286b0c17b53d11bf83..a85c01bb225ab58a372a8b09fb07e7bc155a7aeb 100644 --- a/crates/agent/src/agent.rs +++ b/crates/agent/src/agent.rs @@ -133,9 +133,7 @@ impl LanguageModels { for model in provider.provided_models(cx) { let model_info = Self::map_language_model_to_info(&model, &provider); let model_id = model_info.id.clone(); - if !recommended_models.contains(&(model.provider_id(), model.id())) { - provider_models.push(model_info); - } + provider_models.push(model_info); models.insert(model_id, model); } if !provider_models.is_empty() { diff --git a/crates/agent_ui/src/language_model_selector.rs b/crates/agent_ui/src/language_model_selector.rs index 0f7b83e3edba6c8d97c2c12a939a65cb71c39dca..1de6bee791f782713d869bac7974ad3ec4e08b9f 100644 --- a/crates/agent_ui/src/language_model_selector.rs +++ b/crates/agent_ui/src/language_model_selector.rs @@ -1,6 +1,6 @@ use std::{cmp::Reverse, sync::Arc}; -use collections::{HashSet, IndexMap}; +use collections::IndexMap; use fuzzy::{StringMatch, StringMatchCandidate, match_strings}; use gpui::{Action, AnyElement, App, BackgroundExecutor, DismissEvent, Subscription, Task}; use language_model::{ @@ -57,7 +57,7 @@ fn all_models(cx: &App) -> GroupedModels { }) .collect(); - let other = providers + let all = providers .iter() .flat_map(|provider| { provider @@ -70,7 +70,7 @@ fn all_models(cx: &App) -> GroupedModels { }) .collect(); - GroupedModels::new(other, recommended) + GroupedModels::new(all, recommended) } #[derive(Clone)] @@ -210,33 +210,24 @@ impl LanguageModelPickerDelegate { struct GroupedModels { recommended: Vec, - other: IndexMap>, + all: IndexMap>, } impl GroupedModels { - pub fn new(other: Vec, recommended: Vec) -> Self { - let recommended_ids = recommended - .iter() - .map(|info| (info.model.provider_id(), info.model.id())) - .collect::>(); - - let mut other_by_provider: IndexMap<_, Vec> = IndexMap::default(); - for model in other { - if recommended_ids.contains(&(model.model.provider_id(), model.model.id())) { - continue; - } - + pub fn new(all: Vec, recommended: Vec) -> Self { + let mut all_by_provider: IndexMap<_, Vec> = IndexMap::default(); + for model in all { let provider = model.model.provider_id(); - if let Some(models) = other_by_provider.get_mut(&provider) { + if let Some(models) = all_by_provider.get_mut(&provider) { models.push(model); } else { - other_by_provider.insert(provider, vec![model]); + all_by_provider.insert(provider, vec![model]); } } Self { recommended, - other: other_by_provider, + all: all_by_provider, } } @@ -252,7 +243,7 @@ impl GroupedModels { ); } - for models in self.other.values() { + for models in self.all.values() { if models.is_empty() { continue; } @@ -267,20 +258,6 @@ impl GroupedModels { } entries } - - fn model_infos(&self) -> Vec { - let other = self - .other - .values() - .flat_map(|model| model.iter()) - .cloned() - .collect::>(); - self.recommended - .iter() - .chain(&other) - .cloned() - .collect::>() - } } enum LanguageModelPickerEntry { @@ -425,8 +402,9 @@ impl PickerDelegate for LanguageModelPickerDelegate { .collect::>(); let available_models = all_models - .model_infos() - .iter() + .all + .values() + .flat_map(|models| models.iter()) .filter(|m| configured_provider_ids.contains(&m.model.provider_id())) .cloned() .collect::>(); @@ -764,46 +742,52 @@ mod tests { } #[gpui::test] - fn test_exclude_recommended_models(_cx: &mut TestAppContext) { + fn test_recommended_models_also_appear_in_other(_cx: &mut TestAppContext) { let recommended_models = create_models(vec![("zed", "claude")]); let all_models = create_models(vec![ - ("zed", "claude"), // Should be filtered out from "other" + ("zed", "claude"), // Should also appear in "other" ("zed", "gemini"), ("copilot", "o3"), ]); let grouped_models = GroupedModels::new(all_models, recommended_models); - let actual_other_models = grouped_models - .other + let actual_all_models = grouped_models + .all .values() .flatten() .cloned() .collect::>(); - // Recommended models should not appear in "other" - assert_models_eq(actual_other_models, vec!["zed/gemini", "copilot/o3"]); + // Recommended models should also appear in "all" + assert_models_eq( + actual_all_models, + vec!["zed/claude", "zed/gemini", "copilot/o3"], + ); } #[gpui::test] - fn test_dont_exclude_models_from_other_providers(_cx: &mut TestAppContext) { + fn test_models_from_different_providers(_cx: &mut TestAppContext) { let recommended_models = create_models(vec![("zed", "claude")]); let all_models = create_models(vec![ - ("zed", "claude"), // Should be filtered out from "other" + ("zed", "claude"), // Should also appear in "other" ("zed", "gemini"), - ("copilot", "claude"), // Should not be filtered out from "other" + ("copilot", "claude"), // Different provider, should appear in "other" ]); let grouped_models = GroupedModels::new(all_models, recommended_models); - let actual_other_models = grouped_models - .other + let actual_all_models = grouped_models + .all .values() .flatten() .cloned() .collect::>(); - // Recommended models should not appear in "other" - assert_models_eq(actual_other_models, vec!["zed/gemini", "copilot/claude"]); + // All models should appear in "all" regardless of recommended status + assert_models_eq( + actual_all_models, + vec!["zed/claude", "zed/gemini", "copilot/claude"], + ); } }