From f6160ca5b3da580616883e9039f0e76ad359d515 Mon Sep 17 00:00:00 2001 From: felixxia-oai Date: Tue, 28 Jul 2026 13:11:25 +0000 Subject: [PATCH] Share the skills budget across host and executor catalogs (#35769) ## Why Rendering host and executor skill catalogs independently allows their combined metadata to exceed the skills context budget. ## What changed - Allocate one budget across both catalogs when both contain model-visible skills. - Prefer retaining executor skills when budget pressure requires omissions, while preserving an executor omission marker and reporting when all host skills were omitted. - Evaluate absolute and aliased host paths under the shared budget and select the rendering that best preserves executor entries, total entries, and descriptions, in that order. ## Testing Added renderer and production-turn coverage for shared-budget description shortening, host-first omission, executor omission markers, and alias selection. GitOrigin-RevId: 30041736a76aa173cb641aa98a876a306cb437d2 --- codex-rs/core/tests/suite/skills_extension.rs | 75 +++ codex-rs/ext/skills/src/extension.rs | 144 +++++- codex-rs/ext/skills/src/render.rs | 435 ++++++++++++++---- .../ext/skills/src/render_observability.rs | 7 + codex-rs/ext/skills/src/render_tests.rs | 129 ++++++ codex-rs/ext/skills/src/world_state.rs | 8 + 6 files changed, 688 insertions(+), 110 deletions(-) diff --git a/codex-rs/core/tests/suite/skills_extension.rs b/codex-rs/core/tests/suite/skills_extension.rs index d9b7995164..bae5bc7ef6 100644 --- a/codex-rs/core/tests/suite/skills_extension.rs +++ b/codex-rs/core/tests/suite/skills_extension.rs @@ -127,6 +127,8 @@ const FULL_CATALOG_CONTEXT_WINDOW: i64 = 40_000; const SHORTENING_CONTEXT_WINDOW: i64 = 12_000; const EXECUTOR_OMITTING_CONTEXT_WINDOW: i64 = 2_000; const HOST_OMITTING_CONTEXT_WINDOW: i64 = 2_000; +const MIXED_EXECUTOR_OMITTING_CONTEXT_WINDOW: i64 = 2_000; +const MIXED_HOST_OMITTING_CONTEXT_WINDOW: i64 = 6_000; const HOST_CATALOG: [(&str, &str); 4] = [ ( "host-alpha", @@ -550,6 +552,25 @@ async fn production_turn_shortens_executor_only_catalog_with_the_full_budget() - Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn production_turn_shares_catalog_budget_across_host_and_executor_sections() -> Result<()> { + let (developer_texts, _) = + rendered_catalogs(&HOST_CATALOG, &EXECUTOR_CATALOG, SHORTENING_CONTEXT_WINDOW).await?; + let host_lines = skill_lines(catalog_text(&developer_texts, "host"), "host"); + let executor_lines = skill_lines(catalog_text(&developer_texts, "exec"), "exec"); + let combined_lines = host_lines + .iter() + .chain(executor_lines.iter()) + .copied() + .collect::>(); + + assert_shortened_descriptions(&host_lines, &HOST_CATALOG); + assert_shortened_descriptions(&executor_lines, &EXECUTOR_CATALOG); + assert!(metadata_cost(&combined_lines) <= 240); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn production_turn_keeps_full_host_only_catalog_when_it_fits() -> Result<()> { let (developer_texts, _) = @@ -647,6 +668,60 @@ async fn production_turn_omits_executor_skills_under_extreme_executor_only_press Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn production_turn_omits_executor_skills_after_host_skills_under_extreme_pressure() +-> Result<()> { + let (developer_texts, _) = rendered_catalogs( + &HOST_CATALOG, + &EXECUTOR_CATALOG, + MIXED_EXECUTOR_OMITTING_CONTEXT_WINDOW, + ) + .await?; + let host_lines = developer_texts + .iter() + .flat_map(|text| skill_lines(text, "host")) + .collect::>(); + let executor_text = executor_omission_text(&developer_texts); + let executor_lines = skill_lines(executor_text, "exec"); + + assert_eq!(skill_names(&host_lines), Vec::<&str>::new()); + assert_eq!(skill_names(&executor_lines), vec!["exec-alpha"]); + assert!(executor_text.contains("- 5 additional skills omitted from this bounded skills list.")); + assert!(developer_texts.iter().any(|text| text.contains( + "Host skills are available but omitted from the model-visible skills list because the skills context budget was exceeded." + ))); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn production_turn_omits_host_skills_before_executor_skills_under_extreme_mixed_pressure() +-> Result<()> { + let (developer_texts, warning_messages) = rendered_catalogs( + &HOST_CATALOG, + &EXECUTOR_CATALOG, + MIXED_HOST_OMITTING_CONTEXT_WINDOW, + ) + .await?; + let host_lines = skill_lines(catalog_text(&developer_texts, "host"), "host"); + let executor_lines = skill_lines(catalog_text(&developer_texts, "exec"), "exec"); + + assert_eq!(skill_names(&host_lines), vec!["host-beta"]); + assert_eq!( + skill_names(&executor_lines), + EXECUTOR_CATALOG + .iter() + .map(|(name, _)| *name) + .collect::>() + ); + assert!(warning_messages.contains( + &"Exceeded skills context budget. All skill descriptions were removed and 3 additional skills were not included in the model-visible skills list." + .to_string() + )); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn production_turn_fairly_shortens_extension_catalog_descriptions() -> Result<()> { let server = responses::start_mock_server().await; diff --git a/codex-rs/ext/skills/src/extension.rs b/codex-rs/ext/skills/src/extension.rs index f1583955ca..82dd6f7efa 100644 --- a/codex-rs/ext/skills/src/extension.rs +++ b/codex-rs/ext/skills/src/extension.rs @@ -42,6 +42,7 @@ use crate::fragments::SkillInstructions; use crate::provider::HostSkillProvider; use crate::provider::SkillListQuery; use crate::provider::SkillReadRequest; +use crate::render::AvailableSkillsRender; use crate::render::MAX_SKILL_NAME_BYTES; use crate::render::MAX_SKILL_PATH_BYTES; use crate::render::SkillCatalogRenderPolicy; @@ -49,10 +50,12 @@ use crate::render::SkillMetadataBudget; use crate::render::SkillRenderReport; use crate::render::capped_skill_metadata_budget; use crate::render::render_available_skills; +use crate::render::render_combined_available_skills; use crate::render::truncate_main_prompt_contents; use crate::render::truncate_utf8_to_bytes; use crate::render_observability::CatalogSurface; use crate::render_observability::record_catalog_render; +use crate::render_observability::trace_catalog_budget_pressure; use crate::selection::collect_explicit_skill_mentions; use crate::shadow_selection_experiment::ShadowSelectionExperiment; use crate::sources::SkillProviders; @@ -66,6 +69,7 @@ use crate::warnings::bounded_warnings; use crate::world_state::HostSkillsWarningEmitter; use crate::world_state::executor_skills_world_state_section; use crate::world_state::host_skills_world_state_section; +use crate::world_state::rendered_host_skills_world_state_section; struct SkillsExtension { providers: SkillProviders, @@ -88,7 +92,23 @@ fn render_catalog( policy: SkillCatalogRenderPolicy, budget: SkillMetadataBudget, ) -> RenderedCatalog { - let Some(rendered) = render_available_skills(catalog, policy, budget) else { + render_prepared_catalog( + extension_metrics, + catalog_surface, + include_skills_usage_instructions, + budget, + render_available_skills(catalog, policy, budget), + ) +} + +fn render_prepared_catalog( + extension_metrics: Option<&dyn ExtensionMetrics>, + catalog_surface: CatalogSurface, + include_skills_usage_instructions: bool, + budget: SkillMetadataBudget, + rendered: Option, +) -> RenderedCatalog { + let Some(rendered) = rendered else { record_catalog_render( extension_metrics, catalog_surface, @@ -259,19 +279,78 @@ where .and_then(ModelInfo::resolved_context_window); let metadata_budget = capped_skill_metadata_budget(context_window); let extension_metrics = input.extension_metrics.clone(); - let rendered = if config.include_instructions { - render_catalog( - extension_metrics.as_deref(), - CatalogSurface::ExecutorWorldState, - &catalog, - include_usage, - SkillCatalogRenderPolicy::ExtensionCompatible, - metadata_budget, - ) + let host_snapshot = input.turn_store.get::(); + let shared_rendered = if config.include_instructions + && let Some(host_snapshot) = host_snapshot.as_ref() + && self.providers.has_host_provider() + { + let host_catalog = self + .providers + .list_host_for_turn(SkillListQuery { + turn_id: input.turn_id.to_string(), + executor_roots: Vec::new(), + resolved_executor_roots: Vec::new(), + host_snapshot: Some(Arc::clone(host_snapshot)), + include_host_skills: true, + include_bundled_skills: false, + include_orchestrator_skills: false, + mcp_resources: None, + executor_capability_discovery: None, + }) + .await; + let (host_rendered, executor_rendered) = + render_combined_available_skills(&host_catalog, &catalog, metadata_budget); + if host_catalog + .entries + .iter() + .any(SkillCatalogEntry::is_model_visible) + && catalog + .entries + .iter() + .any(SkillCatalogEntry::is_model_visible) + { + Some((host_rendered, executor_rendered)) + } else { + None + } } else { - RenderedCatalog::default() + None }; - let warning_message = rendered.warning_message; + let (rendered, rendered_host) = + if let Some((host_rendered, executor_rendered)) = shared_rendered { + if let Some(host_rendered) = host_rendered.as_ref() { + trace_catalog_budget_pressure(metadata_budget, &host_rendered.report); + } + ( + render_prepared_catalog( + extension_metrics.as_deref(), + CatalogSurface::ExecutorWorldState, + include_usage, + metadata_budget, + executor_rendered, + ), + host_rendered, + ) + } else { + ( + if config.include_instructions { + render_catalog( + extension_metrics.as_deref(), + CatalogSurface::ExecutorWorldState, + &catalog, + include_usage, + SkillCatalogRenderPolicy::ExtensionCompatible, + metadata_budget, + ) + } else { + RenderedCatalog::default() + }, + None, + ) + }; + let host_warning_message = rendered_host + .as_ref() + .and_then(|rendered_host| rendered_host.report.warning_message()); let executor_body = rendered.fragment.map(|fragment| fragment.body()); let mut sections = vec![executor_skills_world_state_section( executor_body, @@ -280,7 +359,7 @@ where let emitted_warnings = input .turn_store .get_or_init(EmittedCatalogBudgetWarnings::default); - if let Some(host_snapshot) = input.turn_store.get::() + if let Some(host_snapshot) = host_snapshot && self.providers.has_host_provider() { input.turn_store.insert(HostSkillsCatalogInWorldState); @@ -297,16 +376,37 @@ where }); } }); - sections.push(host_skills_world_state_section( - &host_snapshot, - config.include_instructions, - include_usage, - metadata_budget, - extension_metrics, - warning_emitter, - )); + if let Some(rendered_host) = rendered_host { + let report = &rendered_host.report; + let render_metrics = Some(( + report.total_count, + report.included_count, + report.omitted_count, + report.truncated_description_chars, + )); + let body = rendered_host + .into_fragment(include_usage) + .map(|fragment| fragment.body()); + sections.push(rendered_host_skills_world_state_section( + body, + config.include_instructions, + render_metrics, + extension_metrics, + host_warning_message, + warning_emitter, + )); + } else { + sections.push(host_skills_world_state_section( + &host_snapshot, + config.include_instructions, + include_usage, + metadata_budget, + extension_metrics, + warning_emitter, + )); + } } - if let Some(message) = warning_message + if let Some(message) = rendered.warning_message && emitted_warnings.insert(&message) { self.emit_warning(input.thread_store.level_id(), Some(input.turn_id), message); diff --git a/codex-rs/ext/skills/src/render.rs b/codex-rs/ext/skills/src/render.rs index 6e9503c613..24320db96a 100644 --- a/codex-rs/ext/skills/src/render.rs +++ b/codex-rs/ext/skills/src/render.rs @@ -31,13 +31,6 @@ 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, } @@ -256,7 +249,6 @@ impl<'a> SkillLine<'a> { struct RenderedSkillLine { line: String, - truncated_description_chars: usize, } struct RenderedSkillLines { @@ -266,14 +258,13 @@ struct RenderedSkillLines { truncated_description_count: usize, } -struct DescriptionBudgetLine<'a> { - line: &'a SkillLine<'a>, +struct DescriptionBudgetLine { description_char_count: usize, extra_costs: Vec, } -impl<'a> DescriptionBudgetLine<'a> { - fn new(line: &'a SkillLine<'a>, budget: SkillMetadataBudget) -> Self { +impl DescriptionBudgetLine { + fn new(line: &SkillLine<'_>, budget: SkillMetadataBudget) -> Self { let minimum_line = line.render_minimum(); let minimum_chars = minimum_line.chars().count().saturating_add(1); let minimum_bytes = minimum_line.len().saturating_add(1); @@ -297,92 +288,114 @@ impl<'a> DescriptionBudgetLine<'a> { } Self { - line, description_char_count, extra_costs, } } } +#[derive(Clone, Copy)] +enum SkillLineAllocation { + Omitted, + DescriptionChars(usize), +} + fn render_skill_lines( skill_lines: Vec>, budget: SkillMetadataBudget, ) -> RenderedSkillLines { + let allocations = allocate_skill_lines(&skill_lines, budget); + render_allocated_skill_lines(&skill_lines, &allocations) +} + +fn allocate_skill_lines( + skill_lines: &[SkillLine<'_>], + budget: SkillMetadataBudget, +) -> Vec { let full_cost = skill_lines.iter().fold(0usize, |used, line| { used.saturating_add(line.full_cost(budget)) }); if full_cost <= budget.limit() { - 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, - }; + return skill_lines + .iter() + .map(|line| SkillLineAllocation::DescriptionChars(line.description_char_count())) + .collect(); } let minimum_cost = skill_lines.iter().fold(0usize, |used, line| { used.saturating_add(line.minimum_cost(budget)) }); if minimum_cost <= budget.limit() { - let lines = render_lines_with_description_budget( + return allocate_description_chars( budget, - &skill_lines, + 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, - }; + ) + .into_iter() + .map(SkillLineAllocation::DescriptionChars) + .collect(); } - let mut included = Vec::new(); let mut used = 0usize; - let mut omitted = 0usize; + skill_lines + .iter() + .map(|line| { + let next_used = used.saturating_add(line.minimum_cost(budget)); + if next_used <= budget.limit() { + used = next_used; + SkillLineAllocation::DescriptionChars(0) + } else { + SkillLineAllocation::Omitted + } + }) + .collect() +} + +fn render_allocated_skill_lines( + skill_lines: &[SkillLine<'_>], + allocations: &[SkillLineAllocation], +) -> RenderedSkillLines { + let mut lines = Vec::new(); + let mut omitted_count = 0usize; let mut truncated_description_chars = 0usize; let mut truncated_description_count = 0usize; - for line in skill_lines { + for (line, allocation) in skill_lines.iter().zip(allocations) { 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(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); + match allocation { + SkillLineAllocation::Omitted => { + omitted_count = omitted_count.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); + } + } + SkillLineAllocation::DescriptionChars(description_chars) => { + let truncated_chars = description_char_count.saturating_sub(*description_chars); + if truncated_chars > 0 { + truncated_description_chars = + truncated_description_chars.saturating_add(truncated_chars); + truncated_description_count = truncated_description_count.saturating_add(1); + } + lines.push(RenderedSkillLine { + line: line.render_with_description_chars(*description_chars), + }); + } } } RenderedSkillLines { - lines: included, - omitted_count: omitted, + lines, + omitted_count, truncated_description_chars, truncated_description_count, } } -fn render_lines_with_description_budget( +fn allocate_description_chars( budget: SkillMetadataBudget, skill_lines: &[SkillLine<'_>], limit: usize, -) -> Vec { +) -> Vec { let budget_lines = skill_lines .iter() .map(|line| DescriptionBudgetLine::new(line, budget)) @@ -416,31 +429,7 @@ fn render_lines_with_description_budget( } } - budget_lines - .iter() - .zip(char_allocations) - .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), - ) - } - }) + char_allocations } struct RenderedCatalog { @@ -517,6 +506,279 @@ pub(crate) fn render_available_skills( }) } +pub(crate) fn render_combined_available_skills( + host_catalog: &SkillCatalog, + executor_catalog: &SkillCatalog, + budget: SkillMetadataBudget, +) -> (Option, Option) { + let mut host_entries = host_catalog + .entries + .iter() + .filter(|entry| entry.is_model_visible()) + .collect::>(); + let mut executor_entries = executor_catalog + .entries + .iter() + .filter(|entry| entry.is_model_visible()) + .collect::>(); + SkillCatalogRenderPolicy::CoreCompatible.order_entries(&mut host_entries); + SkillCatalogRenderPolicy::ExtensionCompatible.order_entries(&mut executor_entries); + if host_entries.is_empty() || executor_entries.is_empty() { + return ( + render_available_skills( + host_catalog, + SkillCatalogRenderPolicy::CoreCompatible, + budget, + ), + render_available_skills( + executor_catalog, + SkillCatalogRenderPolicy::ExtensionCompatible, + budget, + ), + ); + } + + let absolute = render_combined_lines( + host_entries + .iter() + .map(|entry| SkillLine::new(entry, SkillCatalogRenderPolicy::CoreCompatible)) + .collect(), + executor_entries + .iter() + .map(|entry| SkillLine::new(entry, SkillCatalogRenderPolicy::ExtensionCompatible)) + .collect(), + budget, + Vec::new(), + ); + let selected = if combined_catalog_fully_rendered(&absolute) { + absolute + } else if let Some(aliased) = + build_aliased_combined_catalog(&host_entries, &executor_entries, budget) + && combined_render_is_better(&aliased, &absolute, budget) + { + aliased + } else { + absolute + }; + + (Some(selected.host), Some(selected.executor)) +} + +struct CombinedAvailableSkillsRender { + host: AvailableSkillsRender, + executor: AvailableSkillsRender, +} + +fn render_combined_lines( + host_lines: Vec>, + executor_lines: Vec>, + budget: SkillMetadataBudget, + host_skill_root_lines: Vec, +) -> CombinedAvailableSkillsRender { + let executor_count = executor_lines.len(); + let mut lines = executor_lines; + lines.extend(host_lines); + let mut allocations = allocate_skill_lines(&lines, budget); + let executor_omission_marker = + reserve_executor_omission_marker(&lines, executor_count, budget, &mut allocations); + + CombinedAvailableSkillsRender { + executor: render_combined_group( + &lines[..executor_count], + &allocations[..executor_count], + Vec::new(), + executor_omission_marker, + ), + host: render_combined_group( + &lines[executor_count..], + &allocations[executor_count..], + host_skill_root_lines, + /*omission_marker*/ None, + ), + } +} + +fn render_combined_group( + skill_lines: &[SkillLine<'_>], + allocations: &[SkillLineAllocation], + skill_root_lines: Vec, + omission_marker: Option, +) -> AvailableSkillsRender { + let RenderedSkillLines { + mut lines, + omitted_count, + truncated_description_chars, + truncated_description_count, + } = render_allocated_skill_lines(skill_lines, allocations); + if let Some(marker) = omission_marker { + lines.push(RenderedSkillLine { line: marker }); + } + AvailableSkillsRender { + skill_root_lines, + skill_lines: lines.into_iter().map(|rendered| rendered.line).collect(), + report: SkillRenderReport { + total_count: skill_lines.len(), + included_count: skill_lines.len().saturating_sub(omitted_count), + omitted_count, + truncated_description_chars, + truncated_description_count, + }, + } +} + +fn build_aliased_combined_catalog( + host_entries: &[&SkillCatalogEntry], + executor_entries: &[&SkillCatalogEntry], + budget: SkillMetadataBudget, +) -> Option { + let plan = build_alias_plan(host_entries, budget)?; + if plan.table_cost >= budget.limit() { + return None; + } + + let adjusted_limit = budget.limit().saturating_sub(plan.table_cost); + let adjusted_budget = match budget { + SkillMetadataBudget::Tokens(_) => SkillMetadataBudget::Tokens(adjusted_limit), + SkillMetadataBudget::Characters(_) => SkillMetadataBudget::Characters(adjusted_limit), + }; + let host_lines = host_entries + .iter() + .map(|entry| { + SkillLine::with_locator( + entry, + SkillCatalogRenderPolicy::CoreCompatible, + render_skill_path_with_aliases(entry, &plan), + ) + }) + .collect(); + let executor_lines = executor_entries + .iter() + .map(|entry| SkillLine::new(entry, SkillCatalogRenderPolicy::ExtensionCompatible)) + .collect(); + Some(render_combined_lines( + host_lines, + executor_lines, + adjusted_budget, + plan.skill_root_lines, + )) +} + +fn combined_catalog_fully_rendered(rendered: &CombinedAvailableSkillsRender) -> bool { + rendered.host.report.omitted_count == 0 + && rendered.host.report.truncated_description_chars == 0 + && rendered.executor.report.omitted_count == 0 + && rendered.executor.report.truncated_description_chars == 0 +} + +fn combined_render_is_better( + candidate: &CombinedAvailableSkillsRender, + current: &CombinedAvailableSkillsRender, + budget: SkillMetadataBudget, +) -> bool { + if candidate.executor.report.included_count != current.executor.report.included_count { + return candidate.executor.report.included_count > current.executor.report.included_count; + } + + let included_count = |rendered: &CombinedAvailableSkillsRender| { + rendered + .host + .report + .included_count + .saturating_add(rendered.executor.report.included_count) + }; + if included_count(candidate) != included_count(current) { + return included_count(candidate) > included_count(current); + } + + let truncated_chars = |rendered: &CombinedAvailableSkillsRender| { + rendered + .host + .report + .truncated_description_chars + .saturating_add(rendered.executor.report.truncated_description_chars) + }; + if truncated_chars(candidate) != truncated_chars(current) { + return truncated_chars(candidate) < truncated_chars(current); + } + + combined_available_skills_cost(budget, candidate) + < combined_available_skills_cost(budget, current) +} + +fn combined_available_skills_cost( + budget: SkillMetadataBudget, + rendered: &CombinedAvailableSkillsRender, +) -> usize { + [&rendered.host, &rendered.executor] + .into_iter() + .fold(0usize, |used, catalog| { + let root_cost = if !catalog.skill_root_lines.is_empty() { + aliased_metadata_overhead_cost(budget, &catalog.skill_root_lines) + } else { + Default::default() + }; + catalog + .skill_lines + .iter() + .fold(used.saturating_add(root_cost), |used, line| { + used.saturating_add(metadata_line_cost(budget, line)) + }) + }) +} + +fn reserve_executor_omission_marker( + skill_lines: &[SkillLine<'_>], + executor_count: usize, + budget: SkillMetadataBudget, + allocations: &mut [SkillLineAllocation], +) -> Option { + loop { + let omitted_count = allocations[..executor_count] + .iter() + .filter(|allocation| matches!(allocation, SkillLineAllocation::Omitted)) + .count(); + if omitted_count == 0 { + return None; + } + + let marker = omission_marker(omitted_count); + let used = allocated_skill_lines_cost(skill_lines, allocations, budget); + if used.saturating_add(metadata_line_cost(budget, &marker)) <= budget.limit() { + return Some(marker); + } + + let index = (executor_count..allocations.len()) + .rev() + .chain((0..executor_count).rev()) + .find(|index| { + matches!( + allocations[*index], + SkillLineAllocation::DescriptionChars(_) + ) + })?; + allocations[index] = SkillLineAllocation::Omitted; + } +} + +fn allocated_skill_lines_cost( + skill_lines: &[SkillLine<'_>], + allocations: &[SkillLineAllocation], + budget: SkillMetadataBudget, +) -> usize { + skill_lines + .iter() + .zip(allocations) + .fold(0usize, |used, (line, allocation)| match allocation { + SkillLineAllocation::Omitted => used, + SkillLineAllocation::DescriptionChars(description_chars) => { + used.saturating_add(metadata_line_cost( + budget, + &line.render_with_description_chars(*description_chars), + )) + } + }) +} + fn render_catalog( skill_lines: Vec>, budget: SkillMetadataBudget, @@ -538,10 +800,7 @@ fn render_catalog( loop { let marker = omission_marker(omitted); if total_cost.saturating_add(metadata_line_cost(budget, &marker)) <= budget.limit() { - rendered_lines.push(RenderedSkillLine { - line: marker, - truncated_description_chars: 0, - }); + rendered_lines.push(RenderedSkillLine { line: marker }); break; } let Some(rendered) = rendered_lines.pop() else { diff --git a/codex-rs/ext/skills/src/render_observability.rs b/codex-rs/ext/skills/src/render_observability.rs index 224797a82b..01fddfd5d9 100644 --- a/codex-rs/ext/skills/src/render_observability.rs +++ b/codex-rs/ext/skills/src/render_observability.rs @@ -41,6 +41,13 @@ pub(crate) fn record_catalog_render( report.truncated_description_chars, ); + trace_catalog_budget_pressure(budget, report); +} + +pub(crate) fn trace_catalog_budget_pressure( + budget: SkillMetadataBudget, + report: &SkillRenderReport, +) { if report.omitted_count > 0 || report.truncated_description_chars > 0 { tracing::info!( budget_limit = budget.limit(), diff --git a/codex-rs/ext/skills/src/render_tests.rs b/codex-rs/ext/skills/src/render_tests.rs index 2f5318a635..08b7970baf 100644 --- a/codex-rs/ext/skills/src/render_tests.rs +++ b/codex-rs/ext/skills/src/render_tests.rs @@ -323,6 +323,135 @@ fn mixed_catalogs_keep_absolute_authority_aware_rendering_under_budget_pressure( assert!(!body.contains("additional skills omitted")); } +#[test] +fn mixed_catalog_reserves_executor_omission_marker_by_omitting_host_first() { + let host_catalog = SkillCatalog { + entries: vec![entry_with_path( + "h", "", /*short_description*/ None, "/h", + )], + warnings: Vec::new(), + }; + let executor_entry = |name: &str, resource: &str| { + SkillCatalogEntry::new( + SkillPackageId(name.to_string()), + SkillAuthority::new(SkillSourceKind::Executor, "env-1"), + name, + "", + SkillResourceId::new(resource), + ) + .with_display_path(resource) + }; + let executor_catalog = SkillCatalog { + entries: vec![ + executor_entry("e1", "skill://executor/one"), + executor_entry( + "e2", + "skill://executor/this-resource-is-intentionally-too-long", + ), + ], + warnings: Vec::new(), + }; + + let (host, executor) = render_combined_available_skills( + &host_catalog, + &executor_catalog, + SkillMetadataBudget::Tokens(28), + ); + let host = host.expect("host catalog should render"); + let executor = executor.expect("executor catalog should render"); + + assert_eq!( + host.report, + SkillRenderReport { + total_count: 1, + included_count: 0, + omitted_count: 1, + truncated_description_chars: 0, + truncated_description_count: 0, + } + ); + assert_eq!( + executor.report, + SkillRenderReport { + total_count: 2, + included_count: 1, + omitted_count: 1, + truncated_description_chars: 0, + truncated_description_count: 0, + } + ); + assert_eq!( + executor.skill_lines, + vec![ + "- e1: (environment resource: skill://executor/one)".to_string(), + "- 1 additional skill omitted from this bounded skills list.".to_string(), + ] + ); +} + +#[test] +fn mixed_catalog_prefers_executor_inclusion_over_total_aliased_inclusion() { + let root = format!("/{}", "r".repeat(219)); + let host_catalog = SkillCatalog { + entries: ["h1", "h2"] + .into_iter() + .map(|name| { + entry(name, "", /*short_description*/ None) + .with_display_path(format!("{root}/{name}/SKILL.md")) + .with_display_path_root(root.as_str()) + }) + .collect(), + warnings: Vec::new(), + }; + let executor_entry = |name: &str, resource: &str| { + SkillCatalogEntry::new( + SkillPackageId(name.to_string()), + SkillAuthority::new(SkillSourceKind::Executor, "env-1"), + name, + "", + SkillResourceId::new(resource), + ) + .with_display_path(resource) + }; + let executor_catalog = SkillCatalog { + entries: vec![ + executor_entry("e1", "skill://executor/one"), + executor_entry("e2", &format!("skill://{}", "e".repeat(132))), + ], + warnings: Vec::new(), + }; + + let (host, executor) = render_combined_available_skills( + &host_catalog, + &executor_catalog, + SkillMetadataBudget::Tokens(74), + ); + let host = host.expect("host catalog should render"); + let executor = executor.expect("executor catalog should render"); + + assert_eq!( + executor.report, + SkillRenderReport { + total_count: 2, + included_count: 2, + omitted_count: 0, + truncated_description_chars: 0, + truncated_description_count: 0, + } + ); + assert_eq!( + host.report, + SkillRenderReport { + total_count: 2, + included_count: 0, + omitted_count: 2, + truncated_description_chars: 0, + truncated_description_count: 0, + } + ); + assert_eq!(host.skill_root_lines, Vec::::new()); +} + #[test] fn singleton_plugin_versions_share_the_marketplace_alias_root() { let github_root = "/Users/test/.codex/plugins/cache/openai-curated/github/hash123/skills"; diff --git a/codex-rs/ext/skills/src/world_state.rs b/codex-rs/ext/skills/src/world_state.rs index 35283b404e..1ef108412d 100644 --- a/codex-rs/ext/skills/src/world_state.rs +++ b/codex-rs/ext/skills/src/world_state.rs @@ -25,6 +25,7 @@ const HIDDEN_EXECUTOR_SKILLS_BODY: &str = "\n## Skills update\nSelected-environm const NO_HOST_SKILLS_BODY: &str = "\n## Host skills update\nNo host skills are currently available.\n"; const HIDDEN_HOST_SKILLS_BODY: &str = "\n## Host skills update\nHost skills are not listed automatically. Explicit skill mentions can still be resolved when available.\n"; +const OMITTED_HOST_SKILLS_BODY: &str = "\n## Host skills update\nHost skills are available but omitted from the model-visible skills list because the skills context budget was exceeded.\n"; pub(crate) type HostSkillsWarningEmitter = Arc; @@ -142,6 +143,13 @@ pub(crate) fn rendered_host_skills_world_state_section( warning_message: Option, warning_emitter: HostSkillsWarningEmitter, ) -> WorldStateSectionContribution { + let body = body.or_else(|| { + render_metrics + .is_some_and(|(total_count, included_count, omitted_count, _)| { + total_count > 0 && included_count == 0 && omitted_count > 0 + }) + .then(|| OMITTED_HOST_SKILLS_BODY.to_string()) + }); let snapshot = json!({ "body": body, "includeInstructions": include_instructions,