mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Use model metadata for skills usage instructions (#29740)
## Summary - add a false-by-default `include_skills_usage_instructions` model metadata field - enable the field for the bundled `gpt-5.5` model metadata - consume the metadata in both core and extension skill rendering - remove hardcoded legacy-model matching and its marker plumbing
This commit is contained in:
committed by
GitHub
Unverified
parent
850da19dc4
commit
6b5f5743b3
@@ -34,6 +34,7 @@ fn preset_to_info(preset: &ModelPreset, priority: i32) -> ModelInfo {
|
||||
upgrade: preset.upgrade.as_ref().map(Into::into),
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summaries: false,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
|
||||
@@ -138,7 +138,7 @@ async fn orchestrator_skill_can_read_referenced_resource_without_an_executor() -
|
||||
|
||||
let thread_start_id = mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
model: Some("mock-model".to_string()),
|
||||
model: Some("gpt-5.5".to_string()),
|
||||
environments: Some(Vec::new()),
|
||||
..Default::default()
|
||||
})
|
||||
|
||||
@@ -80,6 +80,7 @@ async fn models_client_hits_models_endpoint() {
|
||||
upgrade: None,
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summaries: false,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
|
||||
@@ -75,14 +75,6 @@ pub fn render_available_skills_body(skill_root_lines: &[String], skill_lines: &[
|
||||
lines.push("### Available skills".to_string());
|
||||
lines.extend(skill_lines.iter().cloned());
|
||||
|
||||
lines.push("### How to use skills".to_string());
|
||||
let how_to_use = if skill_root_lines.is_empty() {
|
||||
SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS
|
||||
} else {
|
||||
SKILLS_HOW_TO_USE_WITH_ALIASES
|
||||
};
|
||||
lines.push(how_to_use.to_string());
|
||||
|
||||
format!("\n{}\n", lines.join("\n"))
|
||||
}
|
||||
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
use codex_core_skills::AvailableSkills;
|
||||
use codex_core_skills::SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS;
|
||||
use codex_core_skills::SKILLS_HOW_TO_USE_WITH_ALIASES;
|
||||
use codex_core_skills::render_available_skills_body;
|
||||
use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG;
|
||||
use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG;
|
||||
@@ -20,13 +22,24 @@ impl AvailableSkillsInstructions {
|
||||
skill_lines,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl From<AvailableSkills> for AvailableSkillsInstructions {
|
||||
fn from(available_skills: AvailableSkills) -> Self {
|
||||
pub fn from_available_skills(
|
||||
available_skills: &AvailableSkills,
|
||||
include_skills_usage_instructions: bool,
|
||||
) -> Self {
|
||||
let mut skill_lines = available_skills.skill_lines.clone();
|
||||
if include_skills_usage_instructions {
|
||||
skill_lines.push("### How to use skills".to_string());
|
||||
let instructions = if available_skills.skill_root_lines.is_empty() {
|
||||
SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS
|
||||
} else {
|
||||
SKILLS_HOW_TO_USE_WITH_ALIASES
|
||||
};
|
||||
skill_lines.push(instructions.to_string());
|
||||
}
|
||||
Self {
|
||||
skill_root_lines: available_skills.skill_root_lines,
|
||||
skill_lines: available_skills.skill_lines,
|
||||
skill_root_lines: available_skills.skill_root_lines.clone(),
|
||||
skill_lines,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3259,7 +3259,10 @@ impl Session {
|
||||
);
|
||||
if let Some(available_skills) = available_skills {
|
||||
let warning_message = available_skills.warning_message.clone();
|
||||
let skills_instructions = AvailableSkillsInstructions::from(available_skills);
|
||||
let skills_instructions = AvailableSkillsInstructions::from_available_skills(
|
||||
&available_skills,
|
||||
turn_context.model_info.include_skills_usage_instructions,
|
||||
);
|
||||
if let Some(warning_message) = warning_message {
|
||||
self.send_event_raw(Event {
|
||||
id: String::new(),
|
||||
|
||||
@@ -701,6 +701,9 @@ impl Session {
|
||||
&per_turn_config.to_models_manager_config(),
|
||||
)
|
||||
.await;
|
||||
self.services
|
||||
.thread_extension_data
|
||||
.insert(model_info.clone());
|
||||
|
||||
let multi_agent_version = match multi_agent_runtime {
|
||||
TurnMultiAgentRuntime::ResolveAndStore => {
|
||||
|
||||
@@ -241,6 +241,7 @@ fn remote_model_with_auto_review_override(slug: &str, review_model: &str) -> Mod
|
||||
upgrade: None,
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summaries: false,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
|
||||
@@ -124,6 +124,7 @@ fn test_model_info(
|
||||
upgrade: None,
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summaries: false,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
@@ -948,6 +949,7 @@ async fn model_switch_to_smaller_model_updates_token_context_window() -> Result<
|
||||
upgrade: None,
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summaries: false,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
|
||||
@@ -352,6 +352,7 @@ fn test_remote_model(slug: &str, priority: i32) -> ModelInfo {
|
||||
upgrade: None,
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summaries: false,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
|
||||
@@ -574,6 +574,7 @@ async fn remote_model_friendly_personality_instructions_with_feature() -> anyhow
|
||||
personality_pragmatic: Some("Pragmatic variant".to_string()),
|
||||
}),
|
||||
}),
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summaries: false,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
@@ -689,6 +690,7 @@ async fn user_turn_personality_remote_model_template_includes_update_message() -
|
||||
personality_pragmatic: Some(remote_pragmatic_message.to_string()),
|
||||
}),
|
||||
}),
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summaries: false,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
|
||||
@@ -485,6 +485,7 @@ async fn remote_models_remote_model_uses_unified_exec() -> Result<()> {
|
||||
upgrade: None,
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summaries: false,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
@@ -738,6 +739,7 @@ async fn remote_models_apply_remote_base_instructions() -> Result<()> {
|
||||
upgrade: None,
|
||||
base_instructions: remote_base.to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summaries: false,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
@@ -1232,6 +1234,7 @@ fn test_remote_model_with_policy(
|
||||
upgrade: None,
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summaries: false,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
|
||||
@@ -1651,6 +1651,7 @@ async fn stdio_image_responses_are_sanitized_for_text_only_model() -> anyhow::Re
|
||||
upgrade: None,
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summaries: false,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
|
||||
@@ -71,6 +71,7 @@ fn test_model_info(
|
||||
upgrade: None,
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summaries: false,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
|
||||
@@ -1374,6 +1374,7 @@ async fn view_image_tool_returns_unsupported_message_for_text_only_model() -> an
|
||||
upgrade: None,
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summaries: false,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
|
||||
@@ -21,6 +21,7 @@ use codex_extension_api::TurnInputContributor;
|
||||
use codex_extension_api::WorldStateContributionInput;
|
||||
use codex_extension_api::WorldStateSectionContribution;
|
||||
use codex_mcp::McpResourceClient;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::protocol::Event;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::WarningEvent;
|
||||
@@ -129,7 +130,10 @@ where
|
||||
for warning in &catalog.warnings {
|
||||
self.emit_warning(thread_store.level_id(), warning.clone());
|
||||
}
|
||||
available_skills_fragment(&catalog)
|
||||
let include_usage = thread_store
|
||||
.get::<ModelInfo>()
|
||||
.is_some_and(|model_info| model_info.include_skills_usage_instructions);
|
||||
available_skills_fragment(&catalog, include_usage)
|
||||
.map(|fragment| PromptFragment::developer_capability(fragment.render()))
|
||||
.into_iter()
|
||||
.collect()
|
||||
@@ -162,9 +166,14 @@ where
|
||||
input
|
||||
.turn_store
|
||||
.insert(ExecutorSkillsStepState(catalog.clone()));
|
||||
let include_usage = input
|
||||
.thread_store
|
||||
.get::<ModelInfo>()
|
||||
.is_some_and(|model_info| model_info.include_skills_usage_instructions);
|
||||
vec![executor_skills_world_state_section(
|
||||
&catalog,
|
||||
config.include_instructions,
|
||||
include_usage,
|
||||
)]
|
||||
})
|
||||
}
|
||||
@@ -239,7 +248,10 @@ where
|
||||
entry.authority.kind != SkillSourceKind::Executor
|
||||
&& entry.authority.kind != SkillSourceKind::Orchestrator
|
||||
});
|
||||
if let Some(fragment) = available_skills_fragment(&turn_catalog) {
|
||||
let include_usage = thread_store
|
||||
.get::<ModelInfo>()
|
||||
.is_some_and(|model_info| model_info.include_skills_usage_instructions);
|
||||
if let Some(fragment) = available_skills_fragment(&turn_catalog, include_usage) {
|
||||
fragments.push(Box::new(fragment));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use codex_core_skills::SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS;
|
||||
use codex_core_skills::render_available_skills_body;
|
||||
use codex_extension_api::ContextualUserFragment;
|
||||
use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG;
|
||||
@@ -9,7 +10,14 @@ pub(crate) struct AvailableSkillsInstructions {
|
||||
}
|
||||
|
||||
impl AvailableSkillsInstructions {
|
||||
pub(crate) fn from_skill_lines(skill_lines: Vec<String>) -> Self {
|
||||
pub(crate) fn from_skill_lines(
|
||||
mut skill_lines: Vec<String>,
|
||||
include_skills_usage_instructions: bool,
|
||||
) -> Self {
|
||||
if include_skills_usage_instructions {
|
||||
skill_lines.push("### How to use skills".to_string());
|
||||
skill_lines.push(SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS.to_string());
|
||||
}
|
||||
Self { skill_lines }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -21,6 +21,7 @@ pub(crate) const MAX_SKILL_PATH_BYTES: usize = 1_024;
|
||||
)]
|
||||
pub(crate) fn available_skills_fragment(
|
||||
catalog: &SkillCatalog,
|
||||
include_skills_usage_instructions: bool,
|
||||
) -> Option<AvailableSkillsInstructions> {
|
||||
let mut total_bytes = 0usize;
|
||||
let mut omitted = 0usize;
|
||||
@@ -56,7 +57,10 @@ pub(crate) fn available_skills_fragment(
|
||||
));
|
||||
}
|
||||
|
||||
Some(AvailableSkillsInstructions::from_skill_lines(skill_lines))
|
||||
Some(AvailableSkillsInstructions::from_skill_lines(
|
||||
skill_lines,
|
||||
include_skills_usage_instructions,
|
||||
))
|
||||
}
|
||||
|
||||
pub(crate) fn truncate_catalog_skill_description(description: &str) -> Cow<'_, str> {
|
||||
|
||||
@@ -17,9 +17,11 @@ const HIDDEN_EXECUTOR_SKILLS_BODY: &str = "\n## Skills update\nSelected-environm
|
||||
pub(crate) fn executor_skills_world_state_section(
|
||||
catalog: &SkillCatalog,
|
||||
include_instructions: bool,
|
||||
include_skills_usage_instructions: bool,
|
||||
) -> WorldStateSectionContribution {
|
||||
let body = if include_instructions {
|
||||
available_skills_fragment(catalog).map(|fragment| fragment.body())
|
||||
available_skills_fragment(catalog, include_skills_usage_instructions)
|
||||
.map(|fragment| fragment.body())
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
@@ -5,7 +5,6 @@ use std::sync::atomic::AtomicUsize;
|
||||
use std::sync::atomic::Ordering;
|
||||
|
||||
use codex_core_skills::HostSkillsSnapshot;
|
||||
use codex_core_skills::SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS;
|
||||
use codex_core_skills::SKILLS_INTRO_WITH_ABSOLUTE_PATHS;
|
||||
use codex_core_skills::SkillLoadOutcome;
|
||||
use codex_core_skills::SkillMetadata;
|
||||
@@ -125,7 +124,7 @@ async fn installed_extension_uses_host_service_snapshot() -> TestResult {
|
||||
.await;
|
||||
|
||||
let expected_catalog = format!(
|
||||
"{SKILLS_INSTRUCTIONS_OPEN_TAG}\n## Skills\n{SKILLS_INTRO_WITH_ABSOLUTE_PATHS}\n### Available skills\n- demo: Demo skill. (file: {skill_prompt_path})\n### How to use skills\n{SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS}\n{SKILLS_INSTRUCTIONS_CLOSE_TAG}"
|
||||
"{SKILLS_INSTRUCTIONS_OPEN_TAG}\n## Skills\n{SKILLS_INTRO_WITH_ABSOLUTE_PATHS}\n### Available skills\n- demo: Demo skill. (file: {skill_prompt_path})\n{SKILLS_INSTRUCTIONS_CLOSE_TAG}"
|
||||
);
|
||||
let expected_skill = format!(
|
||||
"<skill>\n<name>demo</name>\n<path>{skill_prompt_path}</path>\n{DEMO_SKILL_CONTENTS}\n</skill>"
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -82,6 +82,7 @@ pub fn model_info_from_slug(slug: &str) -> ModelInfo {
|
||||
upgrade: None,
|
||||
base_instructions: BASE_INSTRUCTIONS.to_string(),
|
||||
model_messages: local_personality_messages_for_slug(slug),
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summaries: false,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
|
||||
@@ -370,6 +370,8 @@ pub struct ModelInfo {
|
||||
pub base_instructions: String,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub model_messages: Option<ModelMessages>,
|
||||
#[serde(default)]
|
||||
pub include_skills_usage_instructions: bool,
|
||||
pub supports_reasoning_summaries: bool,
|
||||
#[serde(default)]
|
||||
pub default_reasoning_summary: ReasoningSummary,
|
||||
@@ -665,6 +667,7 @@ mod tests {
|
||||
upgrade: None,
|
||||
base_instructions: "base".to_string(),
|
||||
model_messages: spec,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summaries: false,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
@@ -948,6 +951,7 @@ mod tests {
|
||||
.expect("deserialize model info");
|
||||
|
||||
assert_eq!(model.availability_nux, None);
|
||||
assert!(!model.include_skills_usage_instructions);
|
||||
assert!(!model.supports_image_detail_original);
|
||||
assert_eq!(model.web_search_tool_type, WebSearchToolType::Text);
|
||||
assert!(!model.supports_search_tool);
|
||||
|
||||
@@ -27,6 +27,7 @@ fn model_with_shell_type(shell_type: ConfigShellToolType) -> ModelInfo {
|
||||
upgrade: None,
|
||||
base_instructions: String::new(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summaries: false,
|
||||
default_reasoning_summary: Default::default(),
|
||||
support_verbosity: false,
|
||||
|
||||
Reference in New Issue
Block a user