mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
fix: also try matching namespaced prefix for modelinfo candidate (#12658)
#### What Try matching `\w+`-namespaced model after `longest prefix` as heuristic to match `ModelInfo` from list of candidates. This shouldn't regress existing behavior: - `gpt-5.2-codex` -> `gpt-5.2` if `gpt-5.2-codex` not present - `gpt-5.3` -> `gpt-5` if `gpt-5.3` not present - `gpt-9` still doesn't match anything while being more forgiving for custom prefixes: - `oai/gpt-5.3-codex` -> `gpt-5.3-codex` #### Tests Added unit test.
This commit is contained in:
committed by
GitHub
Unverified
parent
74cebceed7
commit
0b6c2e5652
@@ -159,12 +159,33 @@ impl ModelsManager {
|
||||
best
|
||||
}
|
||||
|
||||
/// Retry metadata lookup for a single namespaced slug like `namespace/model-name`.
|
||||
///
|
||||
/// This only strips one leading namespace segment and only when the namespace is ASCII
|
||||
/// alphanumeric/underscore (`\\w+`) to avoid broadly matching arbitrary aliases.
|
||||
fn find_model_by_namespaced_suffix(model: &str, candidates: &[ModelInfo]) -> Option<ModelInfo> {
|
||||
let (namespace, suffix) = model.split_once('/')?;
|
||||
if suffix.contains('/') {
|
||||
return None;
|
||||
}
|
||||
if !namespace
|
||||
.chars()
|
||||
.all(|c| c.is_ascii_alphanumeric() || c == '_')
|
||||
{
|
||||
return None;
|
||||
}
|
||||
Self::find_model_by_longest_prefix(suffix, candidates)
|
||||
}
|
||||
|
||||
fn construct_model_info_from_candidates(
|
||||
model: &str,
|
||||
candidates: &[ModelInfo],
|
||||
config: &Config,
|
||||
) -> ModelInfo {
|
||||
let remote = Self::find_model_by_longest_prefix(model, candidates);
|
||||
// First use the normal longest-prefix match. If that misses, allow a narrowly scoped
|
||||
// retry for namespaced slugs like `custom/gpt-5.3-codex`.
|
||||
let remote = Self::find_model_by_longest_prefix(model, candidates)
|
||||
.or_else(|| Self::find_model_by_namespaced_suffix(model, candidates));
|
||||
let model_info = if let Some(remote) = remote {
|
||||
ModelInfo {
|
||||
slug: model.to_string(),
|
||||
@@ -520,6 +541,58 @@ mod tests {
|
||||
assert!(!model_info.used_fallback_model_metadata);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn get_model_info_matches_namespaced_suffix() {
|
||||
let codex_home = tempdir().expect("temp dir");
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.build()
|
||||
.await
|
||||
.expect("load default test config");
|
||||
let auth_manager =
|
||||
AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
|
||||
let manager = ModelsManager::new(codex_home.path().to_path_buf(), auth_manager, None);
|
||||
let known_slug = manager
|
||||
.get_remote_models()
|
||||
.await
|
||||
.first()
|
||||
.expect("bundled models should include at least one model")
|
||||
.slug
|
||||
.clone();
|
||||
let namespaced_model = format!("custom/{known_slug}");
|
||||
|
||||
let model_info = manager.get_model_info(&namespaced_model, &config).await;
|
||||
|
||||
assert_eq!(model_info.slug, namespaced_model);
|
||||
assert!(!model_info.used_fallback_model_metadata);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn get_model_info_rejects_multi_segment_namespace_suffix_matching() {
|
||||
let codex_home = tempdir().expect("temp dir");
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.build()
|
||||
.await
|
||||
.expect("load default test config");
|
||||
let auth_manager =
|
||||
AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
|
||||
let manager = ModelsManager::new(codex_home.path().to_path_buf(), auth_manager, None);
|
||||
let known_slug = manager
|
||||
.get_remote_models()
|
||||
.await
|
||||
.first()
|
||||
.expect("bundled models should include at least one model")
|
||||
.slug
|
||||
.clone();
|
||||
let namespaced_model = format!("ns1/ns2/{known_slug}");
|
||||
|
||||
let model_info = manager.get_model_info(&namespaced_model, &config).await;
|
||||
|
||||
assert_eq!(model_info.slug, namespaced_model);
|
||||
assert!(model_info.used_fallback_model_metadata);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn refresh_available_models_sorts_by_priority() {
|
||||
let server = MockServer::start().await;
|
||||
|
||||
@@ -195,6 +195,65 @@ async fn remote_models_long_model_slug_is_sent_with_high_reasoning() -> Result<(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn namespaced_model_slug_uses_catalog_metadata_without_fallback_warning() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
|
||||
let server = MockServer::start().await;
|
||||
let requested_model = "custom/gpt-5.2-codex";
|
||||
let response_mock = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let TestCodex {
|
||||
codex, cwd, config, ..
|
||||
} = test_codex()
|
||||
.with_model(requested_model)
|
||||
.build(&server)
|
||||
.await?;
|
||||
|
||||
codex
|
||||
.submit(Op::UserTurn {
|
||||
items: vec![UserInput::Text {
|
||||
text: "check namespaced model metadata".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
cwd: cwd.path().to_path_buf(),
|
||||
approval_policy: config.permissions.approval_policy.value(),
|
||||
sandbox_policy: config.permissions.sandbox_policy.get().clone(),
|
||||
model: requested_model.to_string(),
|
||||
effort: None,
|
||||
summary: config.model_reasoning_summary,
|
||||
collaboration_mode: None,
|
||||
personality: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
let mut fallback_warning_count = 0;
|
||||
loop {
|
||||
let event = wait_for_event(&codex, |_| true).await;
|
||||
match event {
|
||||
EventMsg::Warning(warning)
|
||||
if warning.message.contains("Defaulting to fallback metadata") =>
|
||||
{
|
||||
fallback_warning_count += 1;
|
||||
}
|
||||
EventMsg::TurnComplete(_) => break,
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
let body = response_mock.single_request().body_json();
|
||||
assert_eq!(body["model"].as_str(), Some(requested_model));
|
||||
assert_eq!(fallback_warning_count, 0);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn remote_models_remote_model_uses_unified_exec() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
Reference in New Issue
Block a user