mirror of
https://github.com/openai/codex.git
synced 2026-09-17 12:23:33 +00:00
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
This commit is contained in:
@@ -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?;
|
||||
|
||||
@@ -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::<Config>::with_event_sink(Arc::new(ChannelEventSink(event_tx)));
|
||||
let mut extensions = ExtensionRegistryBuilder::<Config>::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(())
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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::<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),
|
||||
/*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())
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user