mirror of
https://github.com/openai/codex.git
synced 2026-09-10 20:26:47 +00:00
Add compatibility policies for skill catalog rendering (#34611)
## What changed - Parameterize skill catalog rendering with explicit core- and extension-compatible description policies. - Use full skill descriptions for core-compatible output, while extension-compatible output prefers `short_description` and falls back to the full description. - Keep extension prompt and world-state rendering on the extension-compatible policy. ## Testing - Add a renderer test covering description selection and fallback for both policies. GitOrigin-RevId: 5382075f88c25a49a715feb0702a9ab888ec0123
This commit is contained in:
@@ -43,6 +43,7 @@ use crate::provider::SkillListQuery;
|
||||
use crate::provider::SkillReadRequest;
|
||||
use crate::render::MAX_SKILL_NAME_BYTES;
|
||||
use crate::render::MAX_SKILL_PATH_BYTES;
|
||||
use crate::render::SkillCatalogRenderPolicy;
|
||||
use crate::render::available_skills_fragment;
|
||||
use crate::render::truncate_main_prompt_contents;
|
||||
use crate::render::truncate_utf8_to_bytes;
|
||||
@@ -144,10 +145,14 @@ where
|
||||
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()
|
||||
available_skills_fragment(
|
||||
&catalog,
|
||||
include_usage,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
)
|
||||
.map(|fragment| PromptFragment::developer_capability(fragment.render()))
|
||||
.into_iter()
|
||||
.collect()
|
||||
})
|
||||
}
|
||||
|
||||
@@ -329,7 +334,11 @@ where
|
||||
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) {
|
||||
if let Some(fragment) = available_skills_fragment(
|
||||
&turn_catalog,
|
||||
include_usage,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
) {
|
||||
fragments.push(Box::new(fragment));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -14,6 +14,31 @@ const TRUNCATED_SKILL_DESCRIPTION_SUFFIX: &str = "...";
|
||||
pub(crate) const MAX_SKILL_NAME_BYTES: usize = 256;
|
||||
pub(crate) const MAX_SKILL_PATH_BYTES: usize = 1_024;
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub(crate) enum SkillCatalogRenderPolicy {
|
||||
#[cfg_attr(
|
||||
not(test),
|
||||
expect(
|
||||
dead_code,
|
||||
reason = "used by the host renderer compatibility path in a follow-up"
|
||||
)
|
||||
)]
|
||||
CoreCompatible,
|
||||
ExtensionCompatible,
|
||||
}
|
||||
|
||||
impl SkillCatalogRenderPolicy {
|
||||
fn description(self, entry: &SkillCatalogEntry) -> &str {
|
||||
match self {
|
||||
Self::CoreCompatible => entry.description.as_str(),
|
||||
Self::ExtensionCompatible => entry
|
||||
.short_description
|
||||
.as_deref()
|
||||
.unwrap_or(entry.description.as_str()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[tracing::instrument(
|
||||
level = "trace",
|
||||
skip_all,
|
||||
@@ -22,6 +47,7 @@ pub(crate) const MAX_SKILL_PATH_BYTES: usize = 1_024;
|
||||
pub(crate) fn available_skills_fragment(
|
||||
catalog: &SkillCatalog,
|
||||
include_skills_usage_instructions: bool,
|
||||
policy: SkillCatalogRenderPolicy,
|
||||
) -> Option<AvailableSkillsInstructions> {
|
||||
let mut total_bytes = 0usize;
|
||||
let mut omitted = 0usize;
|
||||
@@ -32,10 +58,7 @@ pub(crate) fn available_skills_fragment(
|
||||
.iter()
|
||||
.filter(|entry| entry.enabled && entry.prompt_visible)
|
||||
{
|
||||
let description = entry
|
||||
.short_description
|
||||
.as_deref()
|
||||
.unwrap_or(entry.description.as_str());
|
||||
let description = policy.description(entry);
|
||||
let description = truncate_catalog_skill_description(description);
|
||||
let line = render_skill_line(entry, description.as_ref());
|
||||
let next_bytes = total_bytes.saturating_add(line.len());
|
||||
@@ -107,3 +130,7 @@ pub(crate) fn truncate_utf8_to_bytes(contents: &str, max_bytes: usize) -> (Strin
|
||||
let truncated = take_bytes_at_char_boundary(contents, max_bytes);
|
||||
(truncated.to_string(), truncated.len() < contents.len())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "render_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
67
codex-rs/ext/skills/src/render_tests.rs
Normal file
67
codex-rs/ext/skills/src/render_tests.rs
Normal file
@@ -0,0 +1,67 @@
|
||||
use super::*;
|
||||
use crate::catalog::SkillAuthority;
|
||||
use crate::catalog::SkillPackageId;
|
||||
use crate::catalog::SkillResourceId;
|
||||
use codex_core_skills::render_available_skills_body;
|
||||
use codex_extension_api::ContextualUserFragment;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn entry(name: &str, description: &str, short_description: Option<&str>) -> SkillCatalogEntry {
|
||||
SkillCatalogEntry::new(
|
||||
SkillPackageId(name.to_string()),
|
||||
SkillAuthority::new(SkillSourceKind::Host, "host"),
|
||||
name,
|
||||
description,
|
||||
SkillResourceId::new(format!("/skills/{name}/SKILL.md")),
|
||||
)
|
||||
.with_short_description(short_description.map(str::to_string))
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn description_selection_follows_render_policy() {
|
||||
let catalog = SkillCatalog {
|
||||
entries: vec![
|
||||
entry("shortened", "full description", Some("short description")),
|
||||
entry(
|
||||
"fallback",
|
||||
"fallback description",
|
||||
/*short_description*/ None,
|
||||
),
|
||||
],
|
||||
warnings: Vec::new(),
|
||||
};
|
||||
|
||||
let core = available_skills_fragment(
|
||||
&catalog,
|
||||
/*include_skills_usage_instructions*/ false,
|
||||
SkillCatalogRenderPolicy::CoreCompatible,
|
||||
)
|
||||
.expect("catalog should render");
|
||||
let extension = available_skills_fragment(
|
||||
&catalog,
|
||||
/*include_skills_usage_instructions*/ false,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
)
|
||||
.expect("catalog should render");
|
||||
|
||||
assert_eq!(
|
||||
core.body(),
|
||||
render_available_skills_body(
|
||||
&[],
|
||||
&[
|
||||
"- shortened: full description (file: /skills/shortened/SKILL.md)".to_string(),
|
||||
"- fallback: fallback description (file: /skills/fallback/SKILL.md)".to_string(),
|
||||
],
|
||||
)
|
||||
);
|
||||
assert_eq!(
|
||||
extension.body(),
|
||||
render_available_skills_body(
|
||||
&[],
|
||||
&[
|
||||
"- shortened: short description (file: /skills/shortened/SKILL.md)".to_string(),
|
||||
"- fallback: fallback description (file: /skills/fallback/SKILL.md)".to_string(),
|
||||
],
|
||||
)
|
||||
);
|
||||
}
|
||||
@@ -12,6 +12,7 @@ use serde_json::json;
|
||||
|
||||
use crate::catalog::SkillCatalog;
|
||||
use crate::fragments::AvailableSkillsInstructions;
|
||||
use crate::render::SkillCatalogRenderPolicy;
|
||||
use crate::render::available_skills_fragment;
|
||||
|
||||
pub(crate) const SKILLS_WORLD_STATE_ID: &str = "skills";
|
||||
@@ -31,8 +32,12 @@ pub(crate) fn executor_skills_world_state_section(
|
||||
include_skills_usage_instructions: bool,
|
||||
) -> WorldStateSectionContribution {
|
||||
let body = if include_instructions {
|
||||
available_skills_fragment(catalog, include_skills_usage_instructions)
|
||||
.map(|fragment| fragment.body())
|
||||
available_skills_fragment(
|
||||
catalog,
|
||||
include_skills_usage_instructions,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
)
|
||||
.map(|fragment| fragment.body())
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user