mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
chore: warn when Code Mode lacks model metadata (#29490)
This commit is contained in:
committed by
GitHub
Unverified
parent
b16d2858f5
commit
3310fc8ae5
@@ -0,0 +1,26 @@
|
||||
use codex_features::Feature;
|
||||
use codex_features::Features;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
|
||||
pub(super) fn unsupported_code_mode_warning(
|
||||
model_info: &ModelInfo,
|
||||
features: &Features,
|
||||
) -> Option<String> {
|
||||
let code_mode_enabled =
|
||||
features.enabled(Feature::CodeMode) || features.enabled(Feature::CodeModeOnly);
|
||||
if !code_mode_enabled
|
||||
|| model_info.tool_mode.is_some()
|
||||
|| model_info.used_fallback_model_metadata
|
||||
{
|
||||
return None;
|
||||
}
|
||||
|
||||
let model = &model_info.slug;
|
||||
Some(format!(
|
||||
"Code Mode is enabled in configuration, but model `{model}` does not advertise Code Mode support. This may degrade model performance. Disable `features.code_mode` and `features.code_mode_only`, or select a model whose metadata enables Code Mode."
|
||||
))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "code_mode_warning_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,70 @@
|
||||
use super::unsupported_code_mode_warning;
|
||||
use codex_features::Feature;
|
||||
use codex_features::Features;
|
||||
use codex_models_manager::model_info::model_info_from_slug;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::openai_models::ToolMode;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
const MODEL_SLUG: &str = "test-model";
|
||||
|
||||
fn known_model_info() -> ModelInfo {
|
||||
ModelInfo {
|
||||
used_fallback_model_metadata: false,
|
||||
..model_info_from_slug(MODEL_SLUG)
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn warns_when_code_mode_is_enabled_without_model_selector() {
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::CodeMode);
|
||||
|
||||
assert_eq!(
|
||||
unsupported_code_mode_warning(&known_model_info(), &features),
|
||||
Some(format!(
|
||||
"Code Mode is enabled in configuration, but model `{MODEL_SLUG}` does not advertise Code Mode support. This may degrade model performance. Disable `features.code_mode` and `features.code_mode_only`, or select a model whose metadata enables Code Mode."
|
||||
))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn warns_when_code_mode_only_is_enabled_without_model_selector() {
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::CodeModeOnly);
|
||||
|
||||
assert!(unsupported_code_mode_warning(&known_model_info(), &features).is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn does_not_warn_when_code_mode_is_disabled() {
|
||||
assert_eq!(
|
||||
unsupported_code_mode_warning(&known_model_info(), &Features::with_defaults()),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn does_not_warn_when_model_has_tool_mode_selector() {
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::CodeModeOnly);
|
||||
|
||||
for tool_mode in [ToolMode::Direct, ToolMode::CodeMode, ToolMode::CodeModeOnly] {
|
||||
let model_info = ModelInfo {
|
||||
tool_mode: Some(tool_mode),
|
||||
..known_model_info()
|
||||
};
|
||||
assert_eq!(unsupported_code_mode_warning(&model_info, &features), None);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fallback_metadata_only_uses_existing_warning() {
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::CodeMode);
|
||||
|
||||
assert_eq!(
|
||||
unsupported_code_mode_warning(&model_info_from_slug(MODEL_SLUG), &features),
|
||||
None
|
||||
);
|
||||
}
|
||||
@@ -218,7 +218,7 @@ pub(super) async fn user_input_or_turn_inner(
|
||||
})
|
||||
.await;
|
||||
}
|
||||
sess.maybe_emit_unknown_model_warning_for_turn(current_context.as_ref())
|
||||
sess.maybe_emit_model_warnings_for_turn(current_context.as_ref())
|
||||
.await;
|
||||
match sess
|
||||
.steer_input(
|
||||
@@ -673,7 +673,7 @@ pub async fn review(
|
||||
review_request: ReviewRequest,
|
||||
) {
|
||||
let turn_context = sess.new_default_turn_with_sub_id(sub_id.clone()).await;
|
||||
sess.maybe_emit_unknown_model_warning_for_turn(turn_context.as_ref())
|
||||
sess.maybe_emit_model_warnings_for_turn(turn_context.as_ref())
|
||||
.await;
|
||||
sess.refresh_mcp_servers_if_requested(&turn_context, Some(sess.mcp_elicitation_reviewer()))
|
||||
.await;
|
||||
|
||||
@@ -94,7 +94,7 @@ impl Session {
|
||||
input,
|
||||
));
|
||||
}
|
||||
self.maybe_emit_unknown_model_warning_for_turn(turn_context.as_ref())
|
||||
self.maybe_emit_model_warnings_for_turn(turn_context.as_ref())
|
||||
.await;
|
||||
if self.input_queue.has_trigger_turn_mailbox_items().await {
|
||||
self.clear_reserved_idle_turn(&turn_state).await;
|
||||
|
||||
@@ -208,6 +208,7 @@ use codex_protocol::error::Result as CodexResult;
|
||||
#[cfg(test)]
|
||||
use codex_protocol::exec_output::StreamOutput;
|
||||
|
||||
mod code_mode_warning;
|
||||
mod config_lock;
|
||||
mod handlers;
|
||||
mod inject;
|
||||
@@ -225,6 +226,7 @@ mod token_budget;
|
||||
pub(crate) mod turn;
|
||||
pub(crate) mod turn_context;
|
||||
mod world_state;
|
||||
use self::code_mode_warning::unsupported_code_mode_warning;
|
||||
use self::config_lock::export_config_lock_if_configured;
|
||||
use self::config_lock::validate_config_lock_if_configured;
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -797,7 +797,7 @@ impl Session {
|
||||
turn_context
|
||||
}
|
||||
|
||||
pub(crate) async fn maybe_emit_unknown_model_warning_for_turn(&self, tc: &TurnContext) {
|
||||
pub(crate) async fn maybe_emit_model_warnings_for_turn(&self, tc: &TurnContext) {
|
||||
if tc.model_info.used_fallback_model_metadata {
|
||||
self.send_event(
|
||||
tc,
|
||||
@@ -810,6 +810,13 @@ impl Session {
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
if let Some(message) =
|
||||
unsupported_code_mode_warning(&tc.model_info, tc.config.features.get())
|
||||
{
|
||||
self.send_event(tc, EventMsg::Warning(WarningEvent { message }))
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn new_default_turn(&self) -> Arc<TurnContext> {
|
||||
|
||||
@@ -483,7 +483,7 @@ impl Session {
|
||||
}
|
||||
|
||||
let turn_context = self.new_default_turn_with_sub_id(sub_id).await;
|
||||
self.maybe_emit_unknown_model_warning_for_turn(turn_context.as_ref())
|
||||
self.maybe_emit_model_warnings_for_turn(turn_context.as_ref())
|
||||
.await;
|
||||
self.start_task(turn_context, Vec::new(), RegularTask::new())
|
||||
.await;
|
||||
|
||||
@@ -22,6 +22,7 @@ use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_response_created;
|
||||
use core_test_support::responses::mount_models_once;
|
||||
use core_test_support::responses::mount_sse_once;
|
||||
use core_test_support::responses::mount_sse_sequence;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::submit_thread_settings;
|
||||
@@ -36,6 +37,12 @@ use tokio::time::sleep;
|
||||
const CHILD_MODEL: &str = "test-multi-agent-child";
|
||||
const ROOT_MODEL: &str = "test-multi-agent-root";
|
||||
const ROOT_PROMPT: &str = "spawn a child";
|
||||
const UNSUPPORTED_CODE_MODE_WARNING: &str = "does not advertise Code Mode support";
|
||||
|
||||
struct RemoteModelResponse {
|
||||
body: Value,
|
||||
warnings: Vec<String>,
|
||||
}
|
||||
|
||||
fn remote_model(slug: &str) -> ModelInfo {
|
||||
ModelInfo {
|
||||
@@ -81,10 +88,10 @@ async fn wait_for_model_available(manager: &SharedModelsManager, slug: &str) ->
|
||||
}
|
||||
}
|
||||
|
||||
async fn response_body_for_remote_model(
|
||||
async fn response_for_remote_model(
|
||||
remote_model: ModelInfo,
|
||||
configure: impl FnOnce(&mut Config) + Send + 'static,
|
||||
) -> Result<Value> {
|
||||
) -> Result<RemoteModelResponse> {
|
||||
let server = responses::start_mock_server().await;
|
||||
let model_slug = remote_model.slug.clone();
|
||||
let models_mock = mount_models_once(
|
||||
@@ -133,12 +140,28 @@ async fn response_body_for_remote_model(
|
||||
thread_settings: Default::default(),
|
||||
})
|
||||
.await?;
|
||||
wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::TurnComplete(_))
|
||||
})
|
||||
.await;
|
||||
let mut warnings = Vec::new();
|
||||
loop {
|
||||
match wait_for_event(&test.codex, |_| true).await {
|
||||
EventMsg::Warning(warning) => warnings.push(warning.message),
|
||||
EventMsg::TurnComplete(_) => break,
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
Ok(response_mock.single_request().body_json())
|
||||
Ok(RemoteModelResponse {
|
||||
body: response_mock.single_request().body_json(),
|
||||
warnings,
|
||||
})
|
||||
}
|
||||
|
||||
async fn response_body_for_remote_model(
|
||||
remote_model: ModelInfo,
|
||||
configure: impl FnOnce(&mut Config) + Send + 'static,
|
||||
) -> Result<Value> {
|
||||
Ok(response_for_remote_model(remote_model, configure)
|
||||
.await?
|
||||
.body)
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
@@ -180,6 +203,117 @@ async fn remote_tool_mode_selector_overrides_feature_flags() -> Result<()> {
|
||||
]
|
||||
);
|
||||
|
||||
let unsupported_model = remote_model("test-tool-mode-unsupported");
|
||||
let unsupported_response = response_for_remote_model(unsupported_model, |config| {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::CodeModeOnly)
|
||||
.expect("test config should allow feature update");
|
||||
})
|
||||
.await?;
|
||||
assert!(
|
||||
tool_names(&unsupported_response.body)
|
||||
.iter()
|
||||
.any(|name| name == codex_code_mode::PUBLIC_TOOL_NAME)
|
||||
);
|
||||
assert_eq!(
|
||||
unsupported_response
|
||||
.warnings
|
||||
.iter()
|
||||
.filter(|warning| warning.contains(UNSUPPORTED_CODE_MODE_WARNING))
|
||||
.count(),
|
||||
1
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn unsupported_code_mode_warning_is_emitted_each_turn() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let model_slug = "test-tool-mode-warning-each-turn";
|
||||
let models_mock = mount_models_once(
|
||||
&server,
|
||||
ModelsResponse {
|
||||
models: vec![remote_model(model_slug)],
|
||||
},
|
||||
)
|
||||
.await;
|
||||
let response_mock = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_assistant_message("msg-1", "done"),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-2"),
|
||||
ev_assistant_message("msg-2", "done"),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
let test = test_codex()
|
||||
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
||||
.with_config(|config| {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::CodeMode)
|
||||
.expect("test config should allow feature update");
|
||||
})
|
||||
.build(&server)
|
||||
.await?;
|
||||
let models_manager = test.thread_manager.get_models_manager();
|
||||
let available_model = wait_for_model_available(&models_manager, model_slug).await;
|
||||
assert_eq!(available_model.model, model_slug);
|
||||
assert_eq!(models_mock.requests().len(), 1);
|
||||
|
||||
submit_thread_settings(
|
||||
&test.codex,
|
||||
ThreadSettingsOverrides {
|
||||
model: Some(model_slug.to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
|
||||
let mut warning_counts = Vec::new();
|
||||
for prompt in ["first turn", "second turn"] {
|
||||
test.codex
|
||||
.submit(Op::UserInput {
|
||||
items: vec![UserInput::Text {
|
||||
text: prompt.to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
responsesapi_client_metadata: None,
|
||||
additional_context: Default::default(),
|
||||
thread_settings: Default::default(),
|
||||
})
|
||||
.await?;
|
||||
|
||||
let mut warning_count = 0;
|
||||
loop {
|
||||
match wait_for_event(&test.codex, |_| true).await {
|
||||
EventMsg::Warning(warning)
|
||||
if warning.message.contains(UNSUPPORTED_CODE_MODE_WARNING) =>
|
||||
{
|
||||
warning_count += 1;
|
||||
}
|
||||
EventMsg::TurnComplete(_) => break,
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
warning_counts.push(warning_count);
|
||||
}
|
||||
|
||||
assert_eq!(warning_counts, vec![1, 1]);
|
||||
assert_eq!(response_mock.requests().len(), 2);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
+8
@@ -0,0 +1,8 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/status_and_layout.rs
|
||||
expression: "lines_to_single_string(&cells[0])"
|
||||
---
|
||||
⚠ Code Mode is enabled in configuration, but model `gpt-5.4` does not advertise
|
||||
Code Mode support. This may degrade model performance. Disable
|
||||
`features.code_mode` and `features.code_mode_only`, or select a model whose
|
||||
metadata enables Code Mode.
|
||||
@@ -2079,6 +2079,22 @@ async fn warning_event_adds_warning_history_cell() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn unsupported_code_mode_warning_renders_as_warning_history_cell() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
handle_warning(
|
||||
&mut chat,
|
||||
"Code Mode is enabled in configuration, but model `gpt-5.4` does not advertise Code Mode support. This may degrade model performance. Disable `features.code_mode` and `features.code_mode_only`, or select a model whose metadata enables Code Mode.",
|
||||
);
|
||||
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(cells.len(), 1, "expected one warning history cell");
|
||||
insta::assert_snapshot!(
|
||||
"unsupported_code_mode_warning",
|
||||
lines_to_single_string(&cells[0])
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn repeated_model_metadata_warning_is_hidden_for_same_slug() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
Reference in New Issue
Block a user