mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Enable request_user_input in Default mode (#12735)
## Summary - allow `request_user_input` in Default collaboration mode as well as Plan - update the Default-mode instructions to prefer assumptions first and use `request_user_input` only when a question is unavoidable - update request_user_input and app-server tests to match the new Default-mode behavior - refactor collaboration-mode availability plumbing into `CollaborationModesConfig` for future mode-related flags ## Codex author `codex resume 019c9124-ed28-7c13-96c6-b916b1c97d49`
This commit is contained in:
committed by
GitHub
Unverified
parent
2bd87d1a75
commit
2f4d6ded1d
@@ -8,9 +8,23 @@ const COLLABORATION_MODE_DEFAULT: &str =
|
||||
include_str!("../../templates/collaboration_mode/default.md");
|
||||
const KNOWN_MODE_NAMES_PLACEHOLDER: &str = "{{KNOWN_MODE_NAMES}}";
|
||||
const REQUEST_USER_INPUT_AVAILABILITY_PLACEHOLDER: &str = "{{REQUEST_USER_INPUT_AVAILABILITY}}";
|
||||
const ASKING_QUESTIONS_GUIDANCE_PLACEHOLDER: &str = "{{ASKING_QUESTIONS_GUIDANCE}}";
|
||||
|
||||
pub(crate) fn builtin_collaboration_mode_presets() -> Vec<CollaborationModeMask> {
|
||||
vec![plan_preset(), default_preset()]
|
||||
/// Stores feature flags that control collaboration-mode behavior.
|
||||
///
|
||||
/// Keep mode-related flags here so new collaboration-mode capabilities can be
|
||||
/// added without large cross-cutting diffs to constructor and call-site
|
||||
/// signatures.
|
||||
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
|
||||
pub struct CollaborationModesConfig {
|
||||
/// Enables `request_user_input` availability in Default mode.
|
||||
pub default_mode_request_user_input: bool,
|
||||
}
|
||||
|
||||
pub(crate) fn builtin_collaboration_mode_presets(
|
||||
collaboration_modes_config: CollaborationModesConfig,
|
||||
) -> Vec<CollaborationModeMask> {
|
||||
vec![plan_preset(), default_preset(collaboration_modes_config)]
|
||||
}
|
||||
|
||||
fn plan_preset() -> CollaborationModeMask {
|
||||
@@ -23,26 +37,35 @@ fn plan_preset() -> CollaborationModeMask {
|
||||
}
|
||||
}
|
||||
|
||||
fn default_preset() -> CollaborationModeMask {
|
||||
fn default_preset(collaboration_modes_config: CollaborationModesConfig) -> CollaborationModeMask {
|
||||
CollaborationModeMask {
|
||||
name: ModeKind::Default.display_name().to_string(),
|
||||
mode: Some(ModeKind::Default),
|
||||
model: None,
|
||||
reasoning_effort: None,
|
||||
developer_instructions: Some(Some(default_mode_instructions())),
|
||||
developer_instructions: Some(Some(default_mode_instructions(collaboration_modes_config))),
|
||||
}
|
||||
}
|
||||
|
||||
fn default_mode_instructions() -> String {
|
||||
fn default_mode_instructions(collaboration_modes_config: CollaborationModesConfig) -> String {
|
||||
let known_mode_names = format_mode_names(&TUI_VISIBLE_COLLABORATION_MODES);
|
||||
let request_user_input_availability =
|
||||
request_user_input_availability_message(ModeKind::Default);
|
||||
let request_user_input_availability = request_user_input_availability_message(
|
||||
ModeKind::Default,
|
||||
collaboration_modes_config.default_mode_request_user_input,
|
||||
);
|
||||
let asking_questions_guidance = asking_questions_guidance_message(
|
||||
collaboration_modes_config.default_mode_request_user_input,
|
||||
);
|
||||
COLLABORATION_MODE_DEFAULT
|
||||
.replace(KNOWN_MODE_NAMES_PLACEHOLDER, &known_mode_names)
|
||||
.replace(
|
||||
REQUEST_USER_INPUT_AVAILABILITY_PLACEHOLDER,
|
||||
&request_user_input_availability,
|
||||
)
|
||||
.replace(
|
||||
ASKING_QUESTIONS_GUIDANCE_PLACEHOLDER,
|
||||
&asking_questions_guidance,
|
||||
)
|
||||
}
|
||||
|
||||
fn format_mode_names(modes: &[ModeKind]) -> String {
|
||||
@@ -55,9 +78,14 @@ fn format_mode_names(modes: &[ModeKind]) -> String {
|
||||
}
|
||||
}
|
||||
|
||||
fn request_user_input_availability_message(mode: ModeKind) -> String {
|
||||
fn request_user_input_availability_message(
|
||||
mode: ModeKind,
|
||||
default_mode_request_user_input: bool,
|
||||
) -> String {
|
||||
let mode_name = mode.display_name();
|
||||
if mode.allows_request_user_input() {
|
||||
if mode.allows_request_user_input()
|
||||
|| (default_mode_request_user_input && mode == ModeKind::Default)
|
||||
{
|
||||
format!("The `request_user_input` tool is available in {mode_name} mode.")
|
||||
} else {
|
||||
format!(
|
||||
@@ -66,6 +94,14 @@ fn request_user_input_availability_message(mode: ModeKind) -> String {
|
||||
}
|
||||
}
|
||||
|
||||
fn asking_questions_guidance_message(default_mode_request_user_input: bool) -> String {
|
||||
if default_mode_request_user_input {
|
||||
"In Default mode, strongly prefer making reasonable assumptions and executing the user's request rather than stopping to ask questions. If you absolutely must ask a question because the answer cannot be discovered from local context and a reasonable assumption would be risky, prefer using the `request_user_input` tool rather than writing a multiple choice question as a textual assistant message. Never write a multiple choice question as a textual assistant message.".to_string()
|
||||
} else {
|
||||
"In Default mode, strongly prefer making reasonable assumptions and executing the user's request rather than stopping to ask questions. If you absolutely must ask a question because the answer cannot be discovered from local context and a reasonable assumption would be risky, ask the user directly with a concise plain-text question. Never write a multiple choice question as a textual assistant message.".to_string()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -74,7 +110,10 @@ mod tests {
|
||||
#[test]
|
||||
fn preset_names_use_mode_display_names() {
|
||||
assert_eq!(plan_preset().name, ModeKind::Plan.display_name());
|
||||
assert_eq!(default_preset().name, ModeKind::Default.display_name());
|
||||
assert_eq!(
|
||||
default_preset(CollaborationModesConfig::default()).name,
|
||||
ModeKind::Default.display_name()
|
||||
);
|
||||
assert_eq!(
|
||||
plan_preset().reasoning_effort,
|
||||
Some(Some(ReasoningEffort::Medium))
|
||||
@@ -83,20 +122,38 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn default_mode_instructions_replace_mode_names_placeholder() {
|
||||
let default_instructions = default_preset()
|
||||
.developer_instructions
|
||||
.expect("default preset should include instructions")
|
||||
.expect("default instructions should be set");
|
||||
let default_instructions = default_preset(CollaborationModesConfig {
|
||||
default_mode_request_user_input: true,
|
||||
})
|
||||
.developer_instructions
|
||||
.expect("default preset should include instructions")
|
||||
.expect("default instructions should be set");
|
||||
|
||||
assert!(!default_instructions.contains(KNOWN_MODE_NAMES_PLACEHOLDER));
|
||||
assert!(!default_instructions.contains(REQUEST_USER_INPUT_AVAILABILITY_PLACEHOLDER));
|
||||
assert!(!default_instructions.contains(ASKING_QUESTIONS_GUIDANCE_PLACEHOLDER));
|
||||
|
||||
let known_mode_names = format_mode_names(&TUI_VISIBLE_COLLABORATION_MODES);
|
||||
let expected_snippet = format!("Known mode names are {known_mode_names}.");
|
||||
assert!(default_instructions.contains(&expected_snippet));
|
||||
|
||||
let expected_availability_message =
|
||||
request_user_input_availability_message(ModeKind::Default);
|
||||
request_user_input_availability_message(ModeKind::Default, true);
|
||||
assert!(default_instructions.contains(&expected_availability_message));
|
||||
assert!(default_instructions.contains("prefer using the `request_user_input` tool"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_mode_instructions_use_plain_text_questions_when_feature_disabled() {
|
||||
let default_instructions = default_preset(CollaborationModesConfig::default())
|
||||
.developer_instructions
|
||||
.expect("default preset should include instructions")
|
||||
.expect("default instructions should be set");
|
||||
|
||||
assert!(!default_instructions.contains("prefer using the `request_user_input` tool"));
|
||||
assert!(
|
||||
default_instructions
|
||||
.contains("ask the user directly with a concise plain-text question")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@ use crate::default_client::build_reqwest_client;
|
||||
use crate::error::CodexErr;
|
||||
use crate::error::Result as CoreResult;
|
||||
use crate::model_provider_info::ModelProviderInfo;
|
||||
use crate::models_manager::collaboration_mode_presets::CollaborationModesConfig;
|
||||
use crate::models_manager::collaboration_mode_presets::builtin_collaboration_mode_presets;
|
||||
use crate::models_manager::model_info;
|
||||
use codex_api::ModelsClient;
|
||||
@@ -55,6 +56,7 @@ enum CatalogMode {
|
||||
pub struct ModelsManager {
|
||||
remote_models: RwLock<Vec<ModelInfo>>,
|
||||
catalog_mode: CatalogMode,
|
||||
collaboration_modes_config: CollaborationModesConfig,
|
||||
auth_manager: Arc<AuthManager>,
|
||||
etag: RwLock<Option<String>>,
|
||||
cache_manager: ModelsCacheManager,
|
||||
@@ -71,6 +73,7 @@ impl ModelsManager {
|
||||
codex_home: PathBuf,
|
||||
auth_manager: Arc<AuthManager>,
|
||||
model_catalog: Option<ModelsResponse>,
|
||||
collaboration_modes_config: CollaborationModesConfig,
|
||||
) -> Self {
|
||||
let cache_path = codex_home.join(MODEL_CACHE_FILE);
|
||||
let cache_manager = ModelsCacheManager::new(cache_path, DEFAULT_MODEL_CACHE_TTL);
|
||||
@@ -88,6 +91,7 @@ impl ModelsManager {
|
||||
Self {
|
||||
remote_models: RwLock::new(remote_models),
|
||||
catalog_mode,
|
||||
collaboration_modes_config,
|
||||
auth_manager,
|
||||
etag: RwLock::new(None),
|
||||
cache_manager,
|
||||
@@ -110,7 +114,14 @@ impl ModelsManager {
|
||||
///
|
||||
/// Returns a static set of presets seeded with the configured model.
|
||||
pub fn list_collaboration_modes(&self) -> Vec<CollaborationModeMask> {
|
||||
builtin_collaboration_mode_presets()
|
||||
self.list_collaboration_modes_for_config(self.collaboration_modes_config)
|
||||
}
|
||||
|
||||
pub fn list_collaboration_modes_for_config(
|
||||
&self,
|
||||
collaboration_modes_config: CollaborationModesConfig,
|
||||
) -> Vec<CollaborationModeMask> {
|
||||
builtin_collaboration_mode_presets(collaboration_modes_config)
|
||||
}
|
||||
|
||||
/// Attempt to list models without blocking, using the current cached state.
|
||||
@@ -378,6 +389,7 @@ impl ModelsManager {
|
||||
.unwrap_or_else(|err| panic!("failed to load bundled models.json: {err}")),
|
||||
),
|
||||
catalog_mode: CatalogMode::Default,
|
||||
collaboration_modes_config: CollaborationModesConfig::default(),
|
||||
auth_manager,
|
||||
etag: RwLock::new(None),
|
||||
cache_manager,
|
||||
@@ -504,7 +516,12 @@ mod tests {
|
||||
.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 manager = ModelsManager::new(
|
||||
codex_home.path().to_path_buf(),
|
||||
auth_manager,
|
||||
None,
|
||||
CollaborationModesConfig::default(),
|
||||
);
|
||||
let known_slug = manager
|
||||
.get_remote_models()
|
||||
.await
|
||||
@@ -541,6 +558,7 @@ mod tests {
|
||||
Some(ModelsResponse {
|
||||
models: vec![remote_model("gpt-overlay", "Overlay", 0)],
|
||||
}),
|
||||
CollaborationModesConfig::default(),
|
||||
);
|
||||
|
||||
let model_info = manager
|
||||
@@ -564,7 +582,12 @@ mod tests {
|
||||
.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 manager = ModelsManager::new(
|
||||
codex_home.path().to_path_buf(),
|
||||
auth_manager,
|
||||
None,
|
||||
CollaborationModesConfig::default(),
|
||||
);
|
||||
let known_slug = manager
|
||||
.get_remote_models()
|
||||
.await
|
||||
@@ -590,7 +613,12 @@ mod tests {
|
||||
.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 manager = ModelsManager::new(
|
||||
codex_home.path().to_path_buf(),
|
||||
auth_manager,
|
||||
None,
|
||||
CollaborationModesConfig::default(),
|
||||
);
|
||||
let known_slug = manager
|
||||
.get_remote_models()
|
||||
.await
|
||||
|
||||
Reference in New Issue
Block a user