github/codex
4
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-21 05:22:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

Report skill catalog truncation during rendering (#34785)

Browse Source 
## What changed

- Return a `SkillRenderReport` with total, included, and omitted skill counts,
  plus truncated description counts and character totals.
- Keep fragment construction separate from rendering so callers can inspect the
  report even when the metadata budget is too small to emit a fragment.

## Testing

- Cover partial description truncation, fully omitted entries, and catalogs
  whose omission marker does not fit the budget.

GitOrigin-RevId: e73fdbcf77ac73c7209100ff6c9b5862b6a59a54
This commit is contained in:
felixxia-oai
2026-07-22 16:26:21 +00:00
committed by copyberry
parent 44436fd075
commit fe6aa9d16c
2 changed files with 252 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/ext/skills/src/render.rs
+163 -28
View File
@@ -71,6 +71,15 @@ pub(crate) enum SkillMetadataBudget {
Characters(usize),
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) struct SkillRenderReport {
pub(crate) total_count: usize,
pub(crate) included_count: usize,
pub(crate) omitted_count: usize,
pub(crate) truncated_description_chars: usize,
pub(crate) truncated_description_count: usize,
}
pub(crate) fn capped_skill_metadata_budget(context_window: Option<i64>) -> SkillMetadataBudget {
context_window
.and_then(|window| usize::try_from(window).ok())
@@ -145,6 +154,10 @@ impl<'a> SkillLine<'a> {
metadata_line_cost(budget, &self.render_minimum())
}
fn description_char_count(&self) -> usize {
self.description.chars().count()
}
fn render_full(&self) -> String {
self.render_with_description(self.description.as_ref())
}
@@ -174,6 +187,18 @@ impl<'a> SkillLine<'a> {
}
}
struct RenderedSkillLine {
line: String,
truncated_description_chars: usize,
}
struct RenderedSkillLines {
lines: Vec<RenderedSkillLine>,
omitted_count: usize,
truncated_description_chars: usize,
truncated_description_count: usize,
}
struct DescriptionBudgetLine<'a> {
line: &'a SkillLine<'a>,
description_char_count: usize,
@@ -215,49 +240,82 @@ impl<'a> DescriptionBudgetLine<'a> {
fn render_skill_lines(
skill_lines: Vec<SkillLine<'_>>,
budget: SkillMetadataBudget,
) -> (Vec<String>, usize) {
) -> RenderedSkillLines {
let full_cost = skill_lines.iter().fold(0usize, |used, line| {
used.saturating_add(line.full_cost(budget))
});
if full_cost <= budget.limit() {
return (skill_lines.iter().map(SkillLine::render_full).collect(), 0);
return RenderedSkillLines {
lines: skill_lines
.iter()
.map(|line| RenderedSkillLine {
line: line.render_full(),
truncated_description_chars: 0,
})
.collect(),
omitted_count: 0,
truncated_description_chars: 0,
truncated_description_count: 0,
};
}
let minimum_cost = skill_lines.iter().fold(0usize, |used, line| {
used.saturating_add(line.minimum_cost(budget))
});
if minimum_cost <= budget.limit() {
return (
render_lines_with_description_budget(
budget,
&skill_lines,
budget.limit().saturating_sub(minimum_cost),
),
0,
let lines = render_lines_with_description_budget(
budget,
&skill_lines,
budget.limit().saturating_sub(minimum_cost),
);
let (truncated_description_chars, truncated_description_count) =
sum_description_truncation(&lines);
return RenderedSkillLines {
lines,
omitted_count: 0,
truncated_description_chars,
truncated_description_count,
};
}
let mut included = Vec::new();
let mut used = 0usize;
let mut omitted = 0usize;
let mut truncated_description_chars = 0usize;
let mut truncated_description_count = 0usize;
for line in skill_lines {
let description_char_count = line.description_char_count();
let rendered = line.render_minimum();
let next_used = used.saturating_add(line.minimum_cost(budget));
if next_used <= budget.limit() {
used = next_used;
included.push(rendered);
included.push(RenderedSkillLine {
line: rendered,
truncated_description_chars: description_char_count,
});
} else {
omitted = omitted.saturating_add(1);
}
truncated_description_chars =
truncated_description_chars.saturating_add(description_char_count);
if description_char_count > 0 {
truncated_description_count = truncated_description_count.saturating_add(1);
}
}
RenderedSkillLines {
lines: included,
omitted_count: omitted,
truncated_description_chars,
truncated_description_count,
}
(included, omitted)
}
fn render_lines_with_description_budget(
budget: SkillMetadataBudget,
skill_lines: &[SkillLine<'_>],
limit: usize,
) -> Vec<String> {
) -> Vec<RenderedSkillLine> {
let budget_lines = skill_lines
.iter()
.map(|line| DescriptionBudgetLine::new(line, budget))
@@ -294,21 +352,66 @@ fn render_lines_with_description_budget(
budget_lines
.iter()
.zip(char_allocations)
.map(|(line, description_chars)| line.line.render_with_description_chars(description_chars))
.map(|(line, description_chars)| RenderedSkillLine {
line: line.line.render_with_description_chars(description_chars),
truncated_description_chars: line
.description_char_count
.saturating_sub(description_chars),
})
.collect()
}
fn sum_description_truncation(rendered: &[RenderedSkillLine]) -> (usize, usize) {
rendered
.iter()
.fold((0usize, 0usize), |(chars, count), line| {
if line.truncated_description_chars == 0 {
(chars, count)
} else {
(
chars.saturating_add(line.truncated_description_chars),
count.saturating_add(1),
)
}
})
}
pub(crate) struct AvailableSkillsRender {
skill_lines: Vec<String>,
#[cfg_attr(
not(test),
expect(
dead_code,
reason = "consumed by alias selection and render side effects in follow-ups"
)
)]
pub(crate) report: SkillRenderReport,
}
impl AvailableSkillsRender {
pub(crate) fn into_fragment(
self,
include_skills_usage_instructions: bool,
) -> Option<AvailableSkillsInstructions> {
(!self.skill_lines.is_empty()).then(|| {
AvailableSkillsInstructions::from_skill_lines(
self.skill_lines,
include_skills_usage_instructions,
)
})
}
}
#[tracing::instrument(
level = "trace",
skip_all,
fields(catalog_entry_count = catalog.entries.len())
)]
pub(crate) fn available_skills_fragment(
pub(crate) fn render_available_skills(
catalog: &SkillCatalog,
include_skills_usage_instructions: bool,
policy: SkillCatalogRenderPolicy,
budget: SkillMetadataBudget,
) -> Option<AvailableSkillsInstructions> {
) -> Option<AvailableSkillsRender> {
let mut entries = catalog
.entries
.iter()
@@ -318,33 +421,65 @@ pub(crate) fn available_skills_fragment(
let skill_lines = entries
.iter()
.map(|entry| SkillLine::new(entry, policy))
.collect();
let (mut skill_lines, mut omitted) = render_skill_lines(skill_lines, budget);
let mut total_cost = skill_lines.iter().fold(0usize, |used, line| {
used.saturating_add(metadata_line_cost(budget, line))
.collect::<Vec<_>>();
if skill_lines.is_empty() {
return None;
}
let total_count = skill_lines.len();
let RenderedSkillLines {
lines: mut rendered_lines,
omitted_count: mut omitted,
truncated_description_chars,
truncated_description_count,
} = render_skill_lines(skill_lines, budget);
let mut total_cost = rendered_lines.iter().fold(0usize, |used, rendered| {
used.saturating_add(metadata_line_cost(budget, &rendered.line))
});
if omitted > 0 {
loop {
let marker = omission_marker(omitted);
if total_cost.saturating_add(metadata_line_cost(budget, &marker)) <= budget.limit() {
skill_lines.push(marker);
rendered_lines.push(RenderedSkillLine {
line: marker,
truncated_description_chars: 0,
});
break;
}
let line = skill_lines.pop()?;
total_cost = total_cost.saturating_sub(metadata_line_cost(budget, &line));
let Some(rendered) = rendered_lines.pop() else {
break;
};
total_cost = total_cost.saturating_sub(metadata_line_cost(budget, &rendered.line));
omitted = omitted.saturating_add(1);
}
}
(!skill_lines.is_empty()).then(|| {
AvailableSkillsInstructions::from_skill_lines(
skill_lines,
include_skills_usage_instructions,
)
Some(AvailableSkillsRender {
skill_lines: rendered_lines
.into_iter()
.map(|rendered| rendered.line)
.collect(),
report: SkillRenderReport {
total_count,
included_count: total_count.saturating_sub(omitted),
omitted_count: omitted,
truncated_description_chars,
truncated_description_count,
},
})
}
pub(crate) fn available_skills_fragment(
catalog: &SkillCatalog,
include_skills_usage_instructions: bool,
policy: SkillCatalogRenderPolicy,
budget: SkillMetadataBudget,
) -> Option<AvailableSkillsInstructions> {
render_available_skills(catalog, policy, budget)?
.into_fragment(include_skills_usage_instructions)
}
fn omission_marker(omitted: usize) -> String {
let skill_word = if omitted == 1 { "skill" } else { "skills" };
format!("- {omitted} additional {skill_word} omitted from this bounded skills list.")
codex-rs/ext/skills/src/render_tests.rs
+89 -3
View File
@@ -244,6 +244,44 @@ fn character_fallback_counts_multibyte_metadata_by_characters() {
assert!(!fragment.body().contains("additional skills omitted"));
}
#[test]
fn catalog_report_counts_partial_description_truncation() {
let catalog = SkillCatalog {
entries: vec![entry(
"partial",
"abcdefghij",
/*short_description*/ None,
)],
warnings: Vec::new(),
};
let expected_line = "- partial: abcd (file: /skills/partial/SKILL.md)";
let budget = SkillMetadataBudget::Characters(metadata_line_cost(
SkillMetadataBudget::Characters(usize::MAX),
expected_line,
));
let render = render_available_skills(
&catalog,
SkillCatalogRenderPolicy::ExtensionCompatible,
budget,
)
.expect("catalog should render");
assert_eq!(
render.report,
SkillRenderReport {
total_count: 1,
included_count: 1,
omitted_count: 0,
truncated_description_chars: 6,
truncated_description_count: 1,
}
);
let fragment = render
.into_fragment(/*include_skills_usage_instructions*/ false)
.expect("partial description should render");
assert!(fragment.body().contains(expected_line));
}
#[test]
fn catalog_emits_omission_marker_when_every_minimum_skill_line_exceeds_budget() {
let oversized = entry(
@@ -257,13 +295,25 @@ fn catalog_emits_omission_marker_when_every_minimum_skill_line_exceeds_budget()
warnings: Vec::new(),
};
let fragment = available_skills_fragment(
let render = render_available_skills(
&catalog,
/*include_skills_usage_instructions*/ false,
SkillCatalogRenderPolicy::ExtensionCompatible,
SkillMetadataBudget::Tokens(100),
)
.expect("omission marker should fit");
.expect("catalog should render");
assert_eq!(
render.report,
SkillRenderReport {
total_count: 1,
included_count: 0,
omitted_count: 1,
truncated_description_chars: MAX_CATALOG_SKILL_DESCRIPTION_CHARS,
truncated_description_count: 1,
}
);
let fragment = render
.into_fragment(/*include_skills_usage_instructions*/ false)
.expect("omission marker should fit");
assert!(!fragment.body().contains("- oversized:"));
assert!(
@@ -272,3 +322,39 @@ fn catalog_emits_omission_marker_when_every_minimum_skill_line_exceeds_budget()
.contains("- 1 additional skill omitted from this bounded skills list.")
);
}
#[test]
fn catalog_preserves_report_when_no_fragment_fits_budget() {
let oversized = entry(
"oversized",
&"x".repeat(MAX_CATALOG_SKILL_DESCRIPTION_CHARS),
/*short_description*/ None,
)
.with_display_path(format!("skill://{}", "x".repeat(512)));
let catalog = SkillCatalog {
entries: vec![oversized],
warnings: Vec::new(),
};
let render = render_available_skills(
&catalog,
SkillCatalogRenderPolicy::ExtensionCompatible,
SkillMetadataBudget::Tokens(1),
)
.expect("catalog should produce a report");
assert_eq!(
render.report,
SkillRenderReport {
total_count: 1,
included_count: 0,
omitted_count: 1,
truncated_description_chars: MAX_CATALOG_SKILL_DESCRIPTION_CHARS,
truncated_description_count: 1,
}
);
assert!(
render
.into_fragment(/*include_skills_usage_instructions*/ false)
.is_none()
);
}