Warn when skill catalogs exceed their context budget (#34997)

## Why

Skill catalog rendering can shorten descriptions or omit enabled skills to fit
the model context budget. Surface these reductions so users know when the
model-visible catalog has lost detail.

## What changed

- Emit a warning with the omitted skill count whenever entries do not fit.
- Warn when description shortening averages more than 100 characters per skill.
- Deduplicate executor catalog warnings across repeated world-state builds in a
  turn.

## Testing

- Cover the description-shortening threshold and omission warning text.
- Verify warnings through extension and production-turn catalogs, including
  per-turn deduplication.

GitOrigin-RevId: bd7fc3482e5dfd8c79072f772a2f54aa502478d9
This commit is contained in:
felixxia-oai
2026-07-23 19:44:12 +00:00
committed by copyberry
parent 7bafdada8b
commit 2c92af09cf
7 changed files with 283 additions and 49 deletions
+35 -2
View File
@@ -2,7 +2,10 @@ use std::sync::Arc;
use anyhow::Result;
use codex_core::config::Config;
use codex_extension_api::ExtensionEventSink;
use codex_extension_api::ExtensionRegistryBuilder;
use codex_protocol::protocol::Event;
use codex_protocol::protocol::EventMsg;
use codex_skills_extension::SkillProvider;
use codex_skills_extension::SkillProviderSource;
use codex_skills_extension::SkillProviders;
@@ -34,6 +37,14 @@ struct StaticSkillProvider {
catalog: SkillCatalog,
}
struct ChannelEventSink(std::sync::mpsc::Sender<Event>);
impl ExtensionEventSink for ChannelEventSink {
fn emit(&self, event: Event) {
let _ = self.0.send(event);
}
}
impl SkillProvider for StaticSkillProvider {
fn list(&self, query: SkillListQuery) -> SkillProviderFuture<'_, SkillCatalog> {
// Keep thread context empty so the catalog is exercised through the
@@ -88,7 +99,10 @@ async fn production_turn_scales_extension_catalog_from_resolved_model_window() -
.collect(),
warnings: Vec::new(),
};
let mut extensions = ExtensionRegistryBuilder::<Config>::new();
let (event_tx, event_rx) = std::sync::mpsc::channel();
let mut extensions = ExtensionRegistryBuilder::<Config>::with_event_sink(Arc::new(
ChannelEventSink(event_tx),
));
install_with_providers(
&mut extensions,
SkillProviders::new().with_provider(SkillProviderSource::new(
@@ -139,12 +153,22 @@ async fn production_turn_scales_extension_catalog_from_resolved_model_window() -
.iter()
.filter(|line| line.starts_with("- skill-"))
.count();
let EventMsg::Warning(warning) = event_rx.try_recv()?.msg else {
panic!("expected catalog budget warning");
};
let omitted_count = 400 - included_count;
assert!(catalog_text.contains("additional skills omitted"));
assert!(!catalog_text.contains(
"A description long enough to keep the catalog under sustained budget pressure."
));
assert!(metadata_cost <= expected_budget);
assert_eq!(
warning.message,
format!(
"Exceeded skills context budget. All skill descriptions were removed and {omitted_count} additional skills were not included in the model-visible skills list."
)
);
included_counts.push(included_count);
}
@@ -180,7 +204,9 @@ async fn production_turn_fairly_shortens_extension_catalog_descriptions() -> Res
.collect(),
warnings: Vec::new(),
};
let mut extensions = ExtensionRegistryBuilder::<Config>::new();
let (event_tx, event_rx) = std::sync::mpsc::channel();
let mut extensions =
ExtensionRegistryBuilder::<Config>::with_event_sink(Arc::new(ChannelEventSink(event_tx)));
install_with_providers(
&mut extensions,
SkillProviders::new().with_provider(SkillProviderSource::new(
@@ -232,6 +258,13 @@ 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 EventMsg::Warning(warning) = event_rx.try_recv()?.msg else {
panic!("expected catalog budget 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(())
}
+63 -11
View File
@@ -36,6 +36,7 @@ use crate::catalog::SkillCatalog;
use crate::catalog::SkillCatalogEntry;
use crate::catalog::SkillReadResult;
use crate::catalog::SkillSourceKind;
use crate::fragments::AvailableSkillsInstructions;
use crate::fragments::SkillInstructions;
use crate::provider::HostSkillProvider;
use crate::provider::SkillListQuery;
@@ -43,13 +44,15 @@ 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::SkillMetadataBudget;
use crate::render::capped_skill_metadata_budget;
use crate::render::render_available_skills;
use crate::render::truncate_main_prompt_contents;
use crate::render::truncate_utf8_to_bytes;
use crate::selection::collect_explicit_skill_mentions;
use crate::shadow_selection_experiment::ShadowSelectionExperiment;
use crate::sources::SkillProviders;
use crate::state::EmittedCatalogBudgetWarnings;
use crate::state::ExecutorSkillsStepState;
use crate::state::SkillsSessionState;
use crate::state::SkillsThreadState;
@@ -65,6 +68,29 @@ struct SkillsExtension<C> {
shadow_selection: Arc<ShadowSelectionExperiment>,
}
#[derive(Default)]
struct RenderedCatalog {
fragment: Option<AvailableSkillsInstructions>,
warning_message: Option<String>,
}
fn render_catalog(
catalog: &SkillCatalog,
include_skills_usage_instructions: bool,
policy: SkillCatalogRenderPolicy,
budget: SkillMetadataBudget,
) -> RenderedCatalog {
let Some(rendered) = render_available_skills(catalog, policy, budget) else {
return RenderedCatalog::default();
};
let warning_message = rendered.report.warning_message();
let fragment = rendered.into_fragment(include_skills_usage_instructions);
RenderedCatalog {
fragment,
warning_message,
}
}
impl<C> ThreadLifecycleContributor<C> for SkillsExtension<C>
where
C: Send + Sync + 'static,
@@ -150,15 +176,20 @@ where
let include_usage = thread_store
.get::<ModelInfo>()
.is_some_and(|model_info| model_info.include_skills_usage_instructions);
available_skills_fragment(
let rendered = render_catalog(
&catalog,
include_usage,
SkillCatalogRenderPolicy::ExtensionCompatible,
capped_skill_metadata_budget(/*context_window*/ None),
)
.map(|fragment| PromptFragment::developer_capability(fragment.render()))
.into_iter()
.collect()
);
if let Some(message) = rendered.warning_message {
self.emit_warning(thread_store.level_id(), message);
}
rendered
.fragment
.map(|fragment| PromptFragment::developer_capability(fragment.render()))
.into_iter()
.collect()
})
}
@@ -200,11 +231,28 @@ where
.as_deref()
.and_then(ModelInfo::resolved_context_window);
let metadata_budget = capped_skill_metadata_budget(context_window);
let rendered = if config.include_instructions {
render_catalog(
&catalog,
include_usage,
SkillCatalogRenderPolicy::ExtensionCompatible,
metadata_budget,
)
} else {
RenderedCatalog::default()
};
if let Some(message) = rendered.warning_message
&& input
.turn_store
.get_or_init(EmittedCatalogBudgetWarnings::default)
.insert(&message)
{
self.emit_warning(input.turn_id, message);
}
let executor_body = rendered.fragment.map(|fragment| fragment.body());
let mut sections = vec![executor_skills_world_state_section(
&catalog,
executor_body,
config.include_instructions,
include_usage,
metadata_budget,
)];
if let Some(host_snapshot) = input.turn_store.get::<HostSkillsSnapshot>()
&& self.providers.has_host_provider()
@@ -352,12 +400,16 @@ where
.as_deref()
.and_then(ModelInfo::resolved_context_window);
let metadata_budget = capped_skill_metadata_budget(context_window);
if let Some(fragment) = available_skills_fragment(
let rendered = render_catalog(
&turn_catalog,
include_usage,
SkillCatalogRenderPolicy::ExtensionCompatible,
metadata_budget,
) {
);
if let Some(message) = rendered.warning_message {
self.emit_warning(&input.turn_id, message);
}
if let Some(fragment) = rendered.fragment {
fragments.push(Box::new(fragment));
}
}
+41 -8
View File
@@ -15,7 +15,11 @@ const SKILL_METADATA_CONTEXT_WINDOW_PERCENT: usize = 2;
const MAX_MAIN_PROMPT_BYTES: usize = 8_000;
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;
pub(crate) const MAX_SKILL_PATH_BYTES: usize = 1_024;
@@ -87,6 +91,41 @@ pub(crate) struct SkillRenderReport {
pub(crate) truncated_description_count: usize,
}
impl SkillRenderReport {
pub(crate) fn warning_message(&self) -> Option<String> {
if self.omitted_count > 0 {
let skill_word = if self.omitted_count == 1 {
"skill"
} else {
"skills"
};
let verb = if self.omitted_count == 1 {
"was"
} else {
"were"
};
return Some(format!(
"{} {} additional {} {} not included in the model-visible skills list.",
SKILL_DESCRIPTIONS_REMOVED_WARNING_PREFIX, self.omitted_count, skill_word, verb
));
}
(self.average_truncated_description_chars()
> SKILL_DESCRIPTION_TRUNCATION_WARNING_THRESHOLD_CHARS)
.then(|| SKILL_DESCRIPTION_TRUNCATED_WARNING.to_string())
}
fn average_truncated_description_chars(&self) -> usize {
if self.total_count == 0 || self.truncated_description_chars == 0 {
return 0;
}
self.truncated_description_chars
.saturating_add(self.total_count.saturating_sub(1))
/ self.total_count
}
}
pub(crate) fn capped_skill_metadata_budget(context_window: Option<i64>) -> SkillMetadataBudget {
context_window
.and_then(|window| usize::try_from(window).ok())
@@ -385,13 +424,6 @@ fn sum_description_truncation(rendered: &[RenderedSkillLine]) -> (usize, usize)
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,
}
@@ -477,7 +509,8 @@ pub(crate) fn render_available_skills(
})
}
pub(crate) fn available_skills_fragment(
#[cfg(test)]
fn available_skills_fragment(
catalog: &SkillCatalog,
include_skills_usage_instructions: bool,
policy: SkillCatalogRenderPolicy,
+65
View File
@@ -310,6 +310,13 @@ fn catalog_emits_omission_marker_when_every_minimum_skill_line_exceeds_budget()
truncated_description_chars: MAX_CATALOG_SKILL_DESCRIPTION_CHARS,
truncated_description_count: 1,
};
assert_eq!(
expected_report.warning_message(),
Some(
"Exceeded skills context budget. All skill descriptions were removed and 1 additional skill was not included in the model-visible skills list."
.to_string()
)
);
let core_render = render_available_skills(
&catalog,
SkillCatalogRenderPolicy::CoreCompatible,
@@ -375,3 +382,61 @@ 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::<Vec<_>>();
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),
)
.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())
);
}
+13
View File
@@ -1,4 +1,5 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::future::Future;
use std::sync::Arc;
use std::sync::Mutex;
@@ -337,6 +338,18 @@ impl OrchestratorResourceCache {
}
}
#[derive(Default)]
pub(crate) struct EmittedCatalogBudgetWarnings(Mutex<HashSet<String>>);
impl EmittedCatalogBudgetWarnings {
pub(crate) fn insert(&self, warning: &str) -> bool {
self.0
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.insert(warning.to_string())
}
}
#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub(crate) struct SkillsTurnState {
pub(crate) catalog: SkillCatalog,
+12 -26
View File
@@ -9,11 +9,8 @@ use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG;
use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG;
use serde_json::json;
use crate::catalog::SkillCatalog;
use crate::fragments::AvailableSkillsInstructions;
use crate::render::SkillCatalogRenderPolicy;
use crate::render::SkillMetadataBudget;
use crate::render::available_skills_fragment;
pub(crate) const SKILLS_WORLD_STATE_ID: &str = "skills";
pub(crate) const HOST_SKILLS_WORLD_STATE_ID: &str = "host_skills";
@@ -25,22 +22,9 @@ const NO_HOST_SKILLS_BODY: &str =
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";
pub(crate) fn executor_skills_world_state_section(
catalog: &SkillCatalog,
body: Option<String>,
include_instructions: bool,
include_skills_usage_instructions: bool,
metadata_budget: SkillMetadataBudget,
) -> WorldStateSectionContribution {
let body = if include_instructions {
available_skills_fragment(
catalog,
include_skills_usage_instructions,
SkillCatalogRenderPolicy::ExtensionCompatible,
metadata_budget,
)
.map(|fragment| fragment.body())
} else {
None
};
let snapshot = json!({
"body": body,
"includeInstructions": include_instructions,
@@ -63,16 +47,18 @@ pub(crate) fn executor_skills_world_state_section(
}
let body = match body.as_deref() {
Some(body) => body,
None if previous_is_absent => return None,
None if !include_instructions => HIDDEN_EXECUTOR_SKILLS_BODY,
None => NO_EXECUTOR_SKILLS_BODY,
Some(body) => Some(body),
None if previous_is_absent => None,
None if !include_instructions => Some(HIDDEN_EXECUTOR_SKILLS_BODY),
None => Some(NO_EXECUTOR_SKILLS_BODY),
};
Some(RenderedWorldStateFragment::new(
"developer",
(SKILLS_INSTRUCTIONS_OPEN_TAG, SKILLS_INSTRUCTIONS_CLOSE_TAG),
body,
))
body.map(|body| {
RenderedWorldStateFragment::new(
"developer",
(SKILLS_INSTRUCTIONS_OPEN_TAG, SKILLS_INSTRUCTIONS_CLOSE_TAG),
body,
)
})
})
.with_legacy_matcher(|role, text| {
role == "developer"
+54 -2
View File
@@ -503,7 +503,9 @@ async fn extreme_budget_pressure_removes_descriptions_before_omitting_entries()
list_calls: None,
fail_first_list: false,
}));
let mut builder = ExtensionRegistryBuilder::new();
let (event_tx, event_rx) = std::sync::mpsc::channel();
let mut builder =
ExtensionRegistryBuilder::with_event_sink(Arc::new(ChannelEventSink(event_tx)));
install_with_providers(&mut builder, providers, skills_extension_config);
let registry = builder.build();
let session_store = ExtensionData::new("session");
@@ -537,6 +539,19 @@ async fn extreme_budget_pressure_removes_descriptions_before_omitting_entries()
assert!(!rendered.contains("- skill-199:"));
assert!(!rendered.contains("description-"));
assert!(rendered.contains("additional skills omitted from this bounded skills list"));
let omitted_count = 200 - included_count;
let event = event_rx.try_recv()?;
assert_eq!("thread", event.id);
let EventMsg::Warning(warning) = event.msg else {
panic!("expected catalog budget warning");
};
assert_eq!(
warning.message,
format!(
"Exceeded skills context budget. All skill descriptions were removed and {omitted_count} additional skills were not included in the model-visible skills list."
)
);
assert!(event_rx.try_recv().is_err());
Ok(())
}
@@ -833,7 +848,9 @@ async fn model_context_window_scales_executor_catalog_but_not_thread_catalog() -
list_calls: None,
fail_first_list: false,
}));
let mut builder = ExtensionRegistryBuilder::new();
let (event_tx, event_rx) = std::sync::mpsc::channel();
let mut builder =
ExtensionRegistryBuilder::with_event_sink(Arc::new(ChannelEventSink(event_tx)));
install_with_providers(&mut builder, providers, skills_extension_config);
let registry = builder.build();
let session_store = ExtensionData::new("session");
@@ -886,11 +903,46 @@ async fn model_context_window_scales_executor_catalog_but_not_thread_catalog() -
turn_store: &turn_store,
})
.await;
// Core rebuilds world state before each sampling step.
let _repeated_sections = registry.context_contributors()[0]
.contribute_world_state(WorldStateContributionInput {
thread_id: codex_protocol::ThreadId::new(),
turn_id: "turn-1",
environments: &[],
ready_selected_capability_roots: &selected_roots,
executor_capability_discovery: None,
session_store: &session_store,
thread_store: &thread_store,
turn_store: &turn_store,
})
.await;
let event = event_rx.try_recv()?;
assert_eq!("turn-1", event.id);
let EventMsg::Warning(warning) = event.msg else {
panic!("expected catalog budget warning");
};
assert!(
warning
.message
.starts_with("Exceeded skills context budget.")
);
assert!(
warning
.message
.ends_with("additional skills were not included in the model-visible skills list.")
);
let snapshot = sections[0].snapshot().clone();
let fragment = sections[0]
.render_diff(PreviousWorldStateSection::Absent)
.ok_or("bounded executor catalog should render")?;
assert!(fragment.body().contains("additional skills omitted"));
assert!(!fragment.body().contains("skill-39"));
assert!(
sections[0]
.render_diff(PreviousWorldStateSection::Known(&snapshot))
.is_none()
);
assert!(event_rx.try_recv().is_err());
Ok(())
}