mirror of
https://github.com/openai/codex.git
synced 2026-11-10 21:00:47 +00:00
Preserve first-party catalog display order in app-server model lists (#60525)
## What changed Return first-party provider models in catalog order while retaining priority-based default selection and existing filtering. Apply this ordering only to provider-origin catalogs for the OpenAI provider using Codex backend routes or `model_catalog_url`. Bundled, configured, and third-party catalogs retain priority ordering. ## Testing Add coverage showing that catalog display order can differ from priority order while the recommended model remains the default. Update custom-provider coverage to verify priority ordering with a reversed catalog response. GitOrigin-RevId: 9c2361e070ba424c092a282ca7f1cf9952d270b3
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
//! Loads the model catalog for the current effective provider configuration.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
|
||||
@@ -7,6 +8,8 @@ use codex_config::ConfigLayerStack;
|
||||
use codex_core::ThreadManager;
|
||||
use codex_core::config::Config;
|
||||
use codex_features::Feature;
|
||||
use codex_model_provider_info::OPENAI_PROVIDER_ID;
|
||||
use codex_models_manager::ModelOrigin;
|
||||
use codex_models_manager::manager::RefreshStrategy;
|
||||
use codex_models_manager::manager::SharedModelsManager;
|
||||
use codex_protocol::openai_models::ModelPreset;
|
||||
@@ -144,8 +147,33 @@ impl ModelCatalog {
|
||||
self.config_manager
|
||||
.check_thread_model_provider(&config)
|
||||
.await?;
|
||||
Ok(models_manager
|
||||
.list_models(refresh_strategy, config.http_client_factory())
|
||||
.await)
|
||||
let catalog = models_manager
|
||||
.model_catalog_with_origins(refresh_strategy, config.http_client_factory())
|
||||
.await;
|
||||
// The first-party catalog supplies picker rows in display order. Bundled,
|
||||
// configured and third-party catalogs rely on the manager's priority order.
|
||||
let use_catalog_order = config.model_provider_id == OPENAI_PROVIDER_ID
|
||||
&& (config.model_provider.supports_codex_backend_routes()
|
||||
|| config.model_provider.model_catalog_url.is_some())
|
||||
&& catalog
|
||||
.iter()
|
||||
.all(|entry| entry.origin == ModelOrigin::Provider);
|
||||
let positions = use_catalog_order.then(|| {
|
||||
catalog
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(index, entry)| (entry.slug.clone(), index))
|
||||
.collect::<HashMap<_, _>>()
|
||||
});
|
||||
let models = catalog
|
||||
.into_iter()
|
||||
.map(|entry| Arc::unwrap_or_clone(entry.model))
|
||||
.collect();
|
||||
let mut presets = models_manager.build_available_models(models);
|
||||
if let Some(positions) = positions {
|
||||
// The manager has filtered the models and marked the default using priority.
|
||||
presets.sort_by_key(|preset| positions[&preset.model]);
|
||||
}
|
||||
Ok(presets)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -20,9 +20,11 @@ use codex_app_server_protocol::RequestId;
|
||||
use codex_config::types::AuthCredentialsStoreMode;
|
||||
use codex_login::AuthKeyringBackendKind;
|
||||
use codex_login::login_with_api_key;
|
||||
use codex_models_manager::model_info::model_info_from_slug;
|
||||
use codex_protocol::openai_models::MODEL_SPECIALTY_CYBER;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::openai_models::ModelPreset;
|
||||
use codex_protocol::openai_models::ModelVisibility;
|
||||
use codex_protocol::openai_models::ModelsResponse;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
@@ -376,7 +378,8 @@ async fn list_models_uses_remote_catalog_as_source_of_truth(
|
||||
.and(header("authorization", format!("Bearer {bearer_token}")))
|
||||
.respond_with(
|
||||
ResponseTemplate::new(/*s*/ 200).set_body_json(ModelsResponse {
|
||||
models: remote_models.clone(),
|
||||
// This custom provider expects priority ordering even if the array is unsorted.
|
||||
models: remote_models.iter().rev().cloned().collect(),
|
||||
}),
|
||||
)
|
||||
.expect(1..)
|
||||
@@ -488,6 +491,57 @@ model_catalog_url = "{server_uri}/v1/models"
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn list_models_uses_display_order_and_marks_the_selection_default() -> Result<()> {
|
||||
let home = TempDir::new()?;
|
||||
let backend = MockServer::start().await;
|
||||
let models = [("z-model", 3), ("a-model", 2), ("recommended", 1)]
|
||||
.into_iter()
|
||||
.map(|(slug, priority)| ModelInfo {
|
||||
priority,
|
||||
visibility: ModelVisibility::List,
|
||||
..model_info_from_slug(slug)
|
||||
})
|
||||
.collect();
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/backend-api/codex/models"))
|
||||
.respond_with(ResponseTemplate::new(/*s*/ 200).set_body_json(ModelsResponse { models }))
|
||||
.mount(&backend)
|
||||
.await;
|
||||
write_chatgpt_auth(
|
||||
home.path(),
|
||||
ChatGptAuthFixture::new("chatgpt-access-token").plan_type("team"),
|
||||
AuthCredentialsStoreMode::File,
|
||||
)?;
|
||||
std::fs::write(
|
||||
home.path().join("config.toml"),
|
||||
format!("openai_base_url = \"{}/backend-api/codex\"", backend.uri()),
|
||||
)?;
|
||||
let mut server = TestAppServer::builder()
|
||||
.with_codex_home(home.path())
|
||||
.build_initialized()
|
||||
.await?;
|
||||
let response: ModelListResponse = server
|
||||
.request(|request_id| ClientRequest::ModelList {
|
||||
request_id,
|
||||
params: ModelListParams::default(),
|
||||
})
|
||||
.await?;
|
||||
assert_eq!(
|
||||
response
|
||||
.data
|
||||
.iter()
|
||||
.map(|row| (row.model.as_str(), row.is_default))
|
||||
.collect::<Vec<_>>(),
|
||||
[
|
||||
("z-model", false),
|
||||
("a-model", false),
|
||||
("recommended", true)
|
||||
]
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn list_models_pagination_works() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
Reference in New Issue
Block a user