mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Reduce the surface of collaboration modes (#20149)
Collaboration modes were slightly invasive both into ThreadManager construction and ModelProvider
This commit is contained in:
@@ -8,28 +8,13 @@ use codex_utils_template::Template;
|
||||
use std::sync::LazyLock;
|
||||
|
||||
const KNOWN_MODE_NAMES_TEMPLATE_KEY: &str = "KNOWN_MODE_NAMES";
|
||||
const REQUEST_USER_INPUT_AVAILABILITY_TEMPLATE_KEY: &str = "REQUEST_USER_INPUT_AVAILABILITY";
|
||||
const ASKING_QUESTIONS_GUIDANCE_TEMPLATE_KEY: &str = "ASKING_QUESTIONS_GUIDANCE";
|
||||
static COLLABORATION_MODE_DEFAULT_TEMPLATE: LazyLock<Template> = LazyLock::new(|| {
|
||||
Template::parse(COLLABORATION_MODE_DEFAULT)
|
||||
.unwrap_or_else(|err| panic!("collaboration mode default template must parse: {err}"))
|
||||
});
|
||||
|
||||
/// 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 fn builtin_collaboration_mode_presets(
|
||||
collaboration_modes_config: CollaborationModesConfig,
|
||||
) -> Vec<CollaborationModeMask> {
|
||||
vec![plan_preset(), default_preset(collaboration_modes_config)]
|
||||
pub fn builtin_collaboration_mode_presets() -> Vec<CollaborationModeMask> {
|
||||
vec![plan_preset(), default_preset()]
|
||||
}
|
||||
|
||||
fn plan_preset() -> CollaborationModeMask {
|
||||
@@ -42,37 +27,20 @@ fn plan_preset() -> CollaborationModeMask {
|
||||
}
|
||||
}
|
||||
|
||||
fn default_preset(collaboration_modes_config: CollaborationModesConfig) -> CollaborationModeMask {
|
||||
fn default_preset() -> 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(collaboration_modes_config))),
|
||||
developer_instructions: Some(Some(default_mode_instructions())),
|
||||
}
|
||||
}
|
||||
|
||||
fn default_mode_instructions(collaboration_modes_config: CollaborationModesConfig) -> String {
|
||||
fn default_mode_instructions() -> String {
|
||||
let known_mode_names = format_mode_names(&TUI_VISIBLE_COLLABORATION_MODES);
|
||||
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_TEMPLATE
|
||||
.render([
|
||||
(KNOWN_MODE_NAMES_TEMPLATE_KEY, known_mode_names.as_str()),
|
||||
(
|
||||
REQUEST_USER_INPUT_AVAILABILITY_TEMPLATE_KEY,
|
||||
request_user_input_availability.as_str(),
|
||||
),
|
||||
(
|
||||
ASKING_QUESTIONS_GUIDANCE_TEMPLATE_KEY,
|
||||
asking_questions_guidance.as_str(),
|
||||
),
|
||||
])
|
||||
.render([(KNOWN_MODE_NAMES_TEMPLATE_KEY, known_mode_names.as_str())])
|
||||
.unwrap_or_else(|err| panic!("collaboration mode default template must render: {err}"))
|
||||
}
|
||||
|
||||
@@ -86,30 +54,6 @@ fn format_mode_names(modes: &[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()
|
||||
|| (default_mode_request_user_input && mode == ModeKind::Default)
|
||||
{
|
||||
format!("The `request_user_input` tool is available in {mode_name} mode.")
|
||||
} else {
|
||||
format!(
|
||||
"The `request_user_input` tool is unavailable in {mode_name} mode. If you call it while in {mode_name} mode, it will return an error."
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
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)]
|
||||
#[path = "collaboration_mode_presets_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -4,49 +4,32 @@ use pretty_assertions::assert_eq;
|
||||
#[test]
|
||||
fn preset_names_use_mode_display_names() {
|
||||
assert_eq!(plan_preset().name, ModeKind::Plan.display_name());
|
||||
assert_eq!(
|
||||
default_preset(CollaborationModesConfig::default()).name,
|
||||
ModeKind::Default.display_name()
|
||||
);
|
||||
assert_eq!(default_preset().name, ModeKind::Default.display_name());
|
||||
assert_eq!(plan_preset().model, None);
|
||||
assert_eq!(
|
||||
plan_preset().reasoning_effort,
|
||||
Some(Some(ReasoningEffort::Medium))
|
||||
);
|
||||
assert_eq!(default_preset().model, None);
|
||||
assert_eq!(default_preset().reasoning_effort, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_mode_instructions_replace_mode_names_placeholder() {
|
||||
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");
|
||||
let default_instructions = default_preset()
|
||||
.developer_instructions
|
||||
.expect("default preset should include instructions")
|
||||
.expect("default instructions should be set");
|
||||
|
||||
assert!(!default_instructions.contains("{{KNOWN_MODE_NAMES}}"));
|
||||
assert!(!default_instructions.contains("{{REQUEST_USER_INPUT_AVAILABILITY}}"));
|
||||
assert!(!default_instructions.contains("{{ASKING_QUESTIONS_GUIDANCE}}"));
|
||||
|
||||
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,
|
||||
/*default_mode_request_user_input*/ 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(
|
||||
"Use the `request_user_input` tool only when it is listed in the available tools"
|
||||
));
|
||||
assert!(
|
||||
default_instructions.contains("ask the user directly with a concise plain-text question")
|
||||
);
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
use super::cache::ModelsCacheManager;
|
||||
use crate::collaboration_mode_presets::CollaborationModesConfig;
|
||||
use crate::collaboration_mode_presets::builtin_collaboration_mode_presets;
|
||||
use crate::config::ModelsManagerConfig;
|
||||
use crate::model_info;
|
||||
@@ -180,7 +179,6 @@ pub type SharedModelsManager = Arc<dyn ModelsManager>;
|
||||
#[derive(Debug)]
|
||||
pub struct OpenAiModelsManager {
|
||||
remote_models: RwLock<Vec<ModelInfo>>,
|
||||
collaboration_modes_config: CollaborationModesConfig,
|
||||
etag: RwLock<Option<String>>,
|
||||
cache_manager: ModelsCacheManager,
|
||||
endpoint_client: SharedModelsEndpointClient,
|
||||
@@ -191,7 +189,6 @@ pub struct OpenAiModelsManager {
|
||||
#[derive(Debug)]
|
||||
pub struct StaticModelsManager {
|
||||
remote_models: Vec<ModelInfo>,
|
||||
collaboration_modes_config: CollaborationModesConfig,
|
||||
auth_manager: Option<Arc<AuthManager>>,
|
||||
}
|
||||
|
||||
@@ -201,14 +198,12 @@ impl OpenAiModelsManager {
|
||||
codex_home: PathBuf,
|
||||
endpoint_client: Arc<dyn ModelsEndpointClient>,
|
||||
auth_manager: Option<Arc<AuthManager>>,
|
||||
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);
|
||||
let remote_models = load_remote_models_from_file().unwrap_or_default();
|
||||
Self {
|
||||
remote_models: RwLock::new(remote_models),
|
||||
collaboration_modes_config,
|
||||
etag: RwLock::new(None),
|
||||
cache_manager,
|
||||
endpoint_client,
|
||||
@@ -219,14 +214,9 @@ impl OpenAiModelsManager {
|
||||
|
||||
impl StaticModelsManager {
|
||||
/// Construct a static model manager from an authoritative catalog.
|
||||
pub fn new(
|
||||
auth_manager: Option<Arc<AuthManager>>,
|
||||
model_catalog: ModelsResponse,
|
||||
collaboration_modes_config: CollaborationModesConfig,
|
||||
) -> Self {
|
||||
pub fn new(auth_manager: Option<Arc<AuthManager>>, model_catalog: ModelsResponse) -> Self {
|
||||
Self {
|
||||
remote_models: model_catalog.models,
|
||||
collaboration_modes_config,
|
||||
auth_manager,
|
||||
}
|
||||
}
|
||||
@@ -256,7 +246,7 @@ impl ModelsManager for OpenAiModelsManager {
|
||||
}
|
||||
|
||||
fn list_collaboration_modes(&self) -> Vec<CollaborationModeMask> {
|
||||
builtin_collaboration_mode_presets(self.collaboration_modes_config)
|
||||
builtin_collaboration_mode_presets()
|
||||
}
|
||||
|
||||
async fn refresh_if_new_etag(&self, etag: String) {
|
||||
@@ -391,7 +381,7 @@ impl ModelsManager for StaticModelsManager {
|
||||
}
|
||||
|
||||
fn list_collaboration_modes(&self) -> Vec<CollaborationModeMask> {
|
||||
builtin_collaboration_mode_presets(self.collaboration_modes_config)
|
||||
builtin_collaboration_mode_presets()
|
||||
}
|
||||
|
||||
async fn refresh_if_new_etag(&self, _etag: String) {}
|
||||
|
||||
@@ -187,20 +187,11 @@ fn openai_manager_for_tests_with_auth(
|
||||
endpoint_client: Arc<dyn ModelsEndpointClient>,
|
||||
auth_manager: Option<Arc<AuthManager>>,
|
||||
) -> OpenAiModelsManager {
|
||||
OpenAiModelsManager::new(
|
||||
codex_home,
|
||||
endpoint_client,
|
||||
auth_manager,
|
||||
CollaborationModesConfig::default(),
|
||||
)
|
||||
OpenAiModelsManager::new(codex_home, endpoint_client, auth_manager)
|
||||
}
|
||||
|
||||
fn static_manager_for_tests(model_catalog: ModelsResponse) -> StaticModelsManager {
|
||||
StaticModelsManager::new(
|
||||
/*auth_manager*/ None,
|
||||
model_catalog,
|
||||
CollaborationModesConfig::default(),
|
||||
)
|
||||
StaticModelsManager::new(/*auth_manager*/ None, model_catalog)
|
||||
}
|
||||
|
||||
async fn chatgpt_auth_tokens_for_tests(codex_home: &Path) -> CodexAuth {
|
||||
@@ -735,7 +726,6 @@ async fn static_manager_reads_latest_auth_mode() {
|
||||
ModelsResponse {
|
||||
models: vec![chatgpt_only_model, api_model],
|
||||
},
|
||||
CollaborationModesConfig::default(),
|
||||
);
|
||||
|
||||
let chatgpt_models = manager.list_models(RefreshStrategy::Online).await;
|
||||
|
||||
Reference in New Issue
Block a user