mirror of
https://github.com/openai/codex.git
synced 2026-05-23 12:34:25 +00:00
Allow catalog review model overrides
This commit is contained in:
@@ -659,14 +659,13 @@ pub(super) async fn run_guardian_review_session(
|
||||
fallback
|
||||
}
|
||||
};
|
||||
let use_parent_model_for_auto_review = turn.model_info.auto_review_model_override == Some(true);
|
||||
let preferred_model_id = turn.provider.approval_review_preferred_model();
|
||||
let preferred_model = available_models
|
||||
let model_override = turn.model_info.auto_review_model_override.as_deref();
|
||||
let review_model_id =
|
||||
model_override.unwrap_or_else(|| turn.provider.approval_review_preferred_model());
|
||||
let review_model = available_models
|
||||
.iter()
|
||||
.find(|preset| preset.model == preferred_model_id);
|
||||
let (guardian_model, guardian_reasoning_effort) = if !use_parent_model_for_auto_review
|
||||
&& let Some(preset) = preferred_model
|
||||
{
|
||||
.find(|preset| preset.model == review_model_id);
|
||||
let (guardian_model, guardian_reasoning_effort) = if let Some(preset) = review_model {
|
||||
let reasoning_effort = preferred_reasoning_effort(
|
||||
preset
|
||||
.supported_reasoning_efforts
|
||||
@@ -674,7 +673,7 @@ pub(super) async fn run_guardian_review_session(
|
||||
.any(|effort| effort.effort == codex_protocol::openai_models::ReasoningEffort::Low),
|
||||
Some(preset.default_reasoning_effort),
|
||||
);
|
||||
(preferred_model_id.to_string(), reasoning_effort)
|
||||
(review_model_id.to_string(), reasoning_effort)
|
||||
} else {
|
||||
let reasoning_effort = preferred_reasoning_effort(
|
||||
turn.model_info
|
||||
@@ -684,7 +683,12 @@ pub(super) async fn run_guardian_review_session(
|
||||
turn.reasoning_effort
|
||||
.or(turn.model_info.default_reasoning_level),
|
||||
);
|
||||
(turn.model_info.slug.clone(), reasoning_effort)
|
||||
(
|
||||
model_override
|
||||
.unwrap_or(turn.model_info.slug.as_str())
|
||||
.to_string(),
|
||||
reasoning_effort,
|
||||
)
|
||||
};
|
||||
let guardian_config = build_guardian_review_session_config(
|
||||
turn.config.as_ref(),
|
||||
|
||||
@@ -1226,7 +1226,7 @@ fn guardian_output_schema_requires_only_outcome_and_allows_optional_details() {
|
||||
}
|
||||
|
||||
async fn guardian_request_model_for_auto_review_override(
|
||||
auto_review_model_override: Option<bool>,
|
||||
auto_review_model_override: Option<String>,
|
||||
) -> anyhow::Result<(String, String, String)> {
|
||||
let server = start_mock_server().await;
|
||||
let guardian_assessment = serde_json::json!({
|
||||
@@ -1284,31 +1284,32 @@ async fn guardian_request_model_for_auto_review_override(
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn guardian_review_uses_parent_model_when_model_catalog_override_is_true()
|
||||
async fn guardian_review_uses_model_catalog_override_when_preferred_review_model_exists()
|
||||
-> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let override_model = "guardian-review-model-override".to_string();
|
||||
let (request_model, parent_model, preferred_model) =
|
||||
guardian_request_model_for_auto_review_override(Some(true)).await?;
|
||||
guardian_request_model_for_auto_review_override(Some(override_model.clone())).await?;
|
||||
|
||||
assert_eq!(request_model, parent_model);
|
||||
assert_eq!(request_model, override_model);
|
||||
assert_ne!(request_model, parent_model);
|
||||
assert_ne!(request_model, preferred_model);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn guardian_review_uses_preferred_review_model_without_parent_override() -> anyhow::Result<()>
|
||||
{
|
||||
async fn guardian_review_uses_preferred_review_model_without_model_catalog_override()
|
||||
-> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
for auto_review_model_override in [None, Some(false)] {
|
||||
let (request_model, parent_model, preferred_model) =
|
||||
guardian_request_model_for_auto_review_override(auto_review_model_override).await?;
|
||||
let (request_model, parent_model, preferred_model) =
|
||||
guardian_request_model_for_auto_review_override(/*auto_review_model_override*/ None)
|
||||
.await?;
|
||||
|
||||
assert_eq!(request_model, preferred_model);
|
||||
assert_ne!(request_model, parent_model);
|
||||
}
|
||||
assert_eq!(request_model, preferred_model);
|
||||
assert_ne!(request_model, parent_model);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -40,16 +40,17 @@ use serde_json::json;
|
||||
use wiremock::MockServer;
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn remote_model_override_uses_parent_model_for_strict_auto_review() -> Result<()> {
|
||||
async fn remote_model_override_uses_catalog_model_for_strict_auto_review() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
|
||||
let server = MockServer::start().await;
|
||||
let model = "remote-auto-review-parent";
|
||||
let review_model = "remote-auto-review-reviewer";
|
||||
mount_models_once(
|
||||
&server,
|
||||
ModelsResponse {
|
||||
models: vec![remote_model_with_auto_review_override(model)],
|
||||
models: vec![remote_model_with_auto_review_override(model, review_model)],
|
||||
},
|
||||
)
|
||||
.await;
|
||||
@@ -129,7 +130,10 @@ async fn remote_model_override_uses_parent_model_for_strict_auto_review() -> Res
|
||||
let model_info = models_manager
|
||||
.get_model_info(model, &config.to_models_manager_config())
|
||||
.await;
|
||||
assert_eq!(model_info.auto_review_model_override, Some(true));
|
||||
assert_eq!(
|
||||
model_info.auto_review_model_override,
|
||||
Some(review_model.to_string())
|
||||
);
|
||||
|
||||
core_test_support::submit_thread_settings(
|
||||
&codex,
|
||||
@@ -191,12 +195,15 @@ async fn remote_model_override_uses_parent_model_for_strict_auto_review() -> Res
|
||||
.into_iter()
|
||||
.find(|request| request.body_contains_text("auto-review-model-override.txt"))
|
||||
.expect("expected Guardian request for strict apply_patch");
|
||||
assert_eq!(guardian_request.body_json()["model"].as_str(), Some(model));
|
||||
assert_eq!(
|
||||
guardian_request.body_json()["model"].as_str(),
|
||||
Some(review_model)
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn remote_model_with_auto_review_override(slug: &str) -> ModelInfo {
|
||||
fn remote_model_with_auto_review_override(slug: &str, review_model: &str) -> ModelInfo {
|
||||
ModelInfo {
|
||||
slug: slug.to_string(),
|
||||
display_name: format!("{slug} display"),
|
||||
@@ -212,7 +219,7 @@ fn remote_model_with_auto_review_override(slug: &str) -> ModelInfo {
|
||||
input_modalities: default_input_modalities(),
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
auto_review_model_override: Some(true),
|
||||
auto_review_model_override: Some(review_model.to_string()),
|
||||
priority: 1,
|
||||
additional_speed_tiers: Vec::new(),
|
||||
service_tiers: Vec::new(),
|
||||
|
||||
@@ -313,7 +313,7 @@ pub struct ModelInfo {
|
||||
#[serde(default)]
|
||||
pub supports_search_tool: bool,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub auto_review_model_override: Option<bool>,
|
||||
pub auto_review_model_override: Option<String>,
|
||||
}
|
||||
|
||||
impl ModelInfo {
|
||||
|
||||
Reference in New Issue
Block a user