From 36b84c81ec0184db024cef38f1d6b55ecf276ef0 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Thu, 17 Sep 2026 01:10:00 +0000 Subject: [PATCH] Suppress warnings when skill descriptions are shortened (#46070) ## What changed Stop emitting a warning when skill descriptions are shortened to fit the skills context budget. Continue warning when skills are omitted from the model-visible list. Update the app-server warning test to set `skills.max_context_tokens` to `1_000` and expect the omitted-skills warning. Remove assertions and tests for the description-shortening warning. GitOrigin-RevId: feac10b3d159d73648a786120d477b7fe4e082d7 --- .../tests/suite/v2/executor_skills.rs | 26 ++++---- codex-rs/core/tests/suite/skills_extension.rs | 9 +-- codex-rs/ext/skills/src/render.rs | 6 +- codex-rs/ext/skills/src/render_tests.rs | 59 ------------------- 4 files changed, 17 insertions(+), 83 deletions(-) diff --git a/codex-rs/app-server/tests/suite/v2/executor_skills.rs b/codex-rs/app-server/tests/suite/v2/executor_skills.rs index d49c683cb4..4e5ce64ba6 100644 --- a/codex-rs/app-server/tests/suite/v2/executor_skills.rs +++ b/codex-rs/app-server/tests/suite/v2/executor_skills.rs @@ -384,12 +384,20 @@ stream_max_retries = 0 let request_id = app_server .send_thread_start_request_with_auto_env(ThreadStartParams { model: Some("mock-model".to_string()), - config: matches!( - scenario, + config: match scenario { + ExecutorSkillScenario::VisibleWithBudgetWarning => Some(HashMap::from([( + "skills.max_context_tokens".to_string(), + json!(1_000), + )])), ExecutorSkillScenario::RestrictedPermittedReference - | ExecutorSkillScenario::RestrictedDeniedReference - ) - .then(|| HashMap::from([("tool_output_token_limit".to_string(), json!(250))])), + | ExecutorSkillScenario::RestrictedDeniedReference => Some(HashMap::from([( + "tool_output_token_limit".to_string(), + json!(250), + )])), + ExecutorSkillScenario::ExplicitOnly | ExecutorSkillScenario::RestrictedVisible => { + None + } + }, selected_capability_roots: Some(vec![SelectedCapabilityRoot { id: "demo-plugin@1".to_string(), location: CapabilityRootLocation::Environment { @@ -444,12 +452,8 @@ stream_max_retries = 0 .await?; } if scenario == ExecutorSkillScenario::VisibleWithBudgetWarning { - let is_skills_budget_warning = |message: &str| { - message.starts_with("Exceeded skills context budget.") - || message.starts_with( - "Skill descriptions were shortened to fit the skills context budget.", - ) - }; + let is_skills_budget_warning = + |message: &str| message.starts_with("Exceeded skills context budget."); let warning = timeout(READ_TIMEOUT, async { loop { let warning: WarningNotification = app_server.read_notification("warning").await?; diff --git a/codex-rs/core/tests/suite/skills_extension.rs b/codex-rs/core/tests/suite/skills_extension.rs index d7b2a7973a..204afccffd 100644 --- a/codex-rs/core/tests/suite/skills_extension.rs +++ b/codex-rs/core/tests/suite/skills_extension.rs @@ -3754,9 +3754,7 @@ async fn production_turn_fairly_shortens_extension_catalog_descriptions() -> Res .collect(), warnings: Vec::new(), }; - let (event_tx, event_rx) = std::sync::mpsc::channel(); - let mut extensions = - ExtensionRegistryBuilder::::with_event_sink(Arc::new(ChannelEventSink(event_tx))); + let mut extensions = ExtensionRegistryBuilder::::new(); install_with_providers( &mut extensions, SkillProviders::new().with_provider(SkillProviderSource::new( @@ -3812,11 +3810,6 @@ async fn production_turn_fairly_shortens_extension_catalog_descriptions() -> Res .all(|length| *length > 0 && *length < 1_024) ); assert!(!catalog_text.contains("additional skills omitted")); - let warning = event_rx.try_recv()?.into_warning(); - assert_eq!( - warning.message, - "Skill descriptions were shortened to fit the skills context budget. Codex can still see every skill, but some descriptions are shorter. Disable unused skills or plugins to leave more room for the rest." - ); Ok(()) } diff --git a/codex-rs/ext/skills/src/render.rs b/codex-rs/ext/skills/src/render.rs index 7deb8446de..ba7a4b7e71 100644 --- a/codex-rs/ext/skills/src/render.rs +++ b/codex-rs/ext/skills/src/render.rs @@ -20,9 +20,7 @@ const MAX_SKILL_PROMPT_BYTES: usize = 8_000; const SKILL_METADATA_CONTEXT_WINDOW_PERCENT: usize = 2; const MAX_CATALOG_SKILL_DESCRIPTION_CHARS: usize = 1_024; const TRUNCATED_SKILL_DESCRIPTION_SUFFIX: &str = "..."; -const SKILL_DESCRIPTION_TRUNCATION_WARNING_THRESHOLD_CHARS: usize = 100; const APPROX_BYTES_PER_TOKEN: usize = 4; -const SKILL_DESCRIPTION_TRUNCATED_WARNING: &str = "Skill descriptions were shortened to fit the skills context budget. Codex can still see every skill, but some descriptions are shorter. Disable unused skills or plugins to leave more room for the rest."; const SKILL_DESCRIPTIONS_REMOVED_WARNING_PREFIX: &str = "Exceeded skills context budget. All skill descriptions were removed and"; pub(crate) const MAX_SKILL_NAME_BYTES: usize = 256; @@ -108,9 +106,7 @@ impl SkillRenderReport { )); } - (self.average_truncated_description_chars() - > SKILL_DESCRIPTION_TRUNCATION_WARNING_THRESHOLD_CHARS) - .then(|| SKILL_DESCRIPTION_TRUNCATED_WARNING.to_string()) + None } pub(crate) fn average_truncated_description_chars(&self) -> usize { diff --git a/codex-rs/ext/skills/src/render_tests.rs b/codex-rs/ext/skills/src/render_tests.rs index 06e40f4baf..ed4c72b0b8 100644 --- a/codex-rs/ext/skills/src/render_tests.rs +++ b/codex-rs/ext/skills/src/render_tests.rs @@ -1085,62 +1085,3 @@ fn catalog_preserves_report_when_no_fragment_fits_budget() { .is_none() ); } - -#[test] -fn substantial_description_shortening_emits_warning() { - let catalog = SkillCatalog { - entries: vec![ - entry( - "long-skill", - &"a".repeat(250), - /*short_description*/ None, - ), - entry("empty-skill", "", /*short_description*/ None), - ], - warnings: Vec::new(), - }; - let skill_lines = catalog - .entries - .iter() - .map(|entry| SkillLine::new(entry, SkillCatalogRenderPolicy::ExtensionCompatible)) - .collect::>(); - let minimum_cost = skill_lines.iter().fold(0usize, |used, line| { - used.saturating_add(line.minimum_cost(SkillMetadataBudget::Characters(usize::MAX))) - }); - let render = render_available_skills( - &catalog, - SkillCatalogRenderPolicy::ExtensionCompatible, - SkillMetadataBudget::Characters(minimum_cost + 49), - /*include_skills_usage_instructions*/ false, - ) - .expect("catalog should render"); - - assert_eq!( - render.report.warning_message(), - Some( - "Skill descriptions were shortened to fit the skills context budget. Codex can still see every skill, but some descriptions are shorter. Disable unused skills or plugins to leave more room for the rest." - .to_string() - ) - ); -} - -#[test] -fn substantial_description_shortening_warning_starts_above_threshold() { - let report_at_threshold = SkillRenderReport { - total_count: 2, - included_count: 2, - omitted_count: 0, - truncated_description_chars: 200, - truncated_description_count: 2, - }; - assert_eq!(report_at_threshold.warning_message(), None); - - let report_above_threshold = SkillRenderReport { - truncated_description_chars: 201, - ..report_at_threshold - }; - assert_eq!( - report_above_threshold.warning_message(), - Some(SKILL_DESCRIPTION_TRUNCATED_WARNING.to_string()) - ); -}