mirror of
https://github.com/openai/codex.git
synced 2026-09-09 15:58:47 +00:00
Suppress omission notices in core-compatible skill catalogs (#34797)
## Why Core-compatible skill catalog output should match the core renderer when the metadata budget omits entries. ## What changed - Emit the bounded-catalog omission notice only for `ExtensionCompatible` rendering. - Continue tracking omitted entries in `SkillRenderReport` for both policies, even when `CoreCompatible` produces no fragment because every entry is omitted. ## Testing Cover policy-specific omission notices and the case where no minimum skill line fits within the metadata budget. GitOrigin-RevId: 79883c15a49c02ff83444ddc272030cb4ddb53c0
This commit is contained in:
@@ -63,6 +63,13 @@ impl SkillCatalogRenderPolicy {
|
||||
Self::ExtensionCompatible => {}
|
||||
}
|
||||
}
|
||||
|
||||
fn includes_omission_notice(self) -> bool {
|
||||
match self {
|
||||
Self::CoreCompatible => false,
|
||||
Self::ExtensionCompatible => true,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
@@ -437,7 +444,7 @@ pub(crate) fn render_available_skills(
|
||||
used.saturating_add(metadata_line_cost(budget, &rendered.line))
|
||||
});
|
||||
|
||||
if omitted > 0 {
|
||||
if omitted > 0 && policy.includes_omission_notice() {
|
||||
loop {
|
||||
let marker = omission_marker(omitted);
|
||||
if total_cost.saturating_add(metadata_line_cost(budget, &marker)) <= budget.limit() {
|
||||
|
||||
@@ -181,7 +181,7 @@ fn catalog_budget_uses_capped_context_percentage_or_character_fallback() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn omission_marker_is_charged_to_catalog_budget() {
|
||||
fn omission_notice_follows_render_policy_and_is_charged_to_catalog_budget() {
|
||||
let catalog = SkillCatalog {
|
||||
entries: (0..20)
|
||||
.map(|index| {
|
||||
@@ -194,6 +194,13 @@ fn omission_marker_is_charged_to_catalog_budget() {
|
||||
.collect(),
|
||||
warnings: Vec::new(),
|
||||
};
|
||||
let core_fragment = available_skills_fragment(
|
||||
&catalog,
|
||||
/*include_skills_usage_instructions*/ false,
|
||||
SkillCatalogRenderPolicy::CoreCompatible,
|
||||
SkillMetadataBudget::Tokens(100),
|
||||
)
|
||||
.expect("core-compatible catalog should render");
|
||||
let fragment = available_skills_fragment(
|
||||
&catalog,
|
||||
/*include_skills_usage_instructions*/ false,
|
||||
@@ -208,6 +215,7 @@ fn omission_marker_is_charged_to_catalog_budget() {
|
||||
.map(|line| approx_token_count(&format!("{line}\n")))
|
||||
.sum::<usize>();
|
||||
|
||||
assert!(!core_fragment.body().contains("additional skills omitted"));
|
||||
assert!(fragment.body().contains("additional skills omitted"));
|
||||
assert!(rendered_metadata_cost <= 100);
|
||||
}
|
||||
@@ -295,22 +303,31 @@ fn catalog_emits_omission_marker_when_every_minimum_skill_line_exceeds_budget()
|
||||
warnings: Vec::new(),
|
||||
};
|
||||
|
||||
let expected_report = SkillRenderReport {
|
||||
total_count: 1,
|
||||
included_count: 0,
|
||||
omitted_count: 1,
|
||||
truncated_description_chars: MAX_CATALOG_SKILL_DESCRIPTION_CHARS,
|
||||
truncated_description_count: 1,
|
||||
};
|
||||
let core_render = render_available_skills(
|
||||
&catalog,
|
||||
SkillCatalogRenderPolicy::CoreCompatible,
|
||||
SkillMetadataBudget::Tokens(100),
|
||||
)
|
||||
.expect("core-compatible report should render");
|
||||
assert_eq!(core_render.report, expected_report);
|
||||
assert_eq!(
|
||||
core_render.into_fragment(/*include_skills_usage_instructions*/ false),
|
||||
None
|
||||
);
|
||||
let render = render_available_skills(
|
||||
&catalog,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
SkillMetadataBudget::Tokens(100),
|
||||
)
|
||||
.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,
|
||||
}
|
||||
);
|
||||
assert_eq!(render.report, expected_report);
|
||||
let fragment = render
|
||||
.into_fragment(/*include_skills_usage_instructions*/ false)
|
||||
.expect("omission marker should fit");
|
||||
|
||||
Reference in New Issue
Block a user