Skip to content

Commit 6f328d5

Browse files
authored
Soften skill description budget warnings (#20112)
Updates skill description budget messaging to be less alarming
1 parent e6db1a9 commit 6f328d5

4 files changed

Lines changed: 53 additions & 29 deletions

File tree

codex-rs/app-server/tests/suite/v2/turn_start.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ async fn turn_start_emits_thread_scoped_warning_notification_for_trimmed_skills(
333333
assert_eq!(warning.thread_id.as_deref(), Some(thread.id.as_str()));
334334
assert_eq!(
335335
warning.message,
336-
"Warning: Exceeded skills context budget of 2%. All skill descriptions were removed and 7 additional skills were not included in the model-visible skills list."
336+
"Exceeded skills context budget of 2%. All skill descriptions were removed and 7 additional skills were not included in the model-visible skills list."
337337
);
338338

339339
timeout(

codex-rs/core-skills/src/render.rs

Lines changed: 47 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,12 @@ use codex_utils_output_truncation::approx_token_count;
1616

1717
const DEFAULT_SKILL_METADATA_CHAR_BUDGET: usize = 8_000;
1818
const SKILL_METADATA_CONTEXT_WINDOW_PERCENT: usize = 2;
19-
const SKILL_DESCRIPTION_TRUNCATION_WARNING_THRESHOLD_CHARS: usize = 10;
19+
const SKILL_DESCRIPTION_TRUNCATION_WARNING_THRESHOLD_CHARS: usize = 100;
2020
const APPROX_BYTES_PER_TOKEN: usize = 4;
21-
pub const SKILL_DESCRIPTION_TRUNCATED_WARNING_PREFIX: &str = "Warning: Exceeded skills context budget. Loaded skill descriptions were truncated by an average of";
21+
pub 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.";
22+
pub const SKILL_DESCRIPTION_TRUNCATED_WARNING_WITH_PERCENT: &str = "Skill descriptions were shortened to fit the 2% 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.";
2223
pub const SKILL_DESCRIPTIONS_REMOVED_WARNING_PREFIX: &str =
23-
"Warning: Exceeded skills context budget. All skill descriptions were removed and";
24+
"Exceeded skills context budget. All skill descriptions were removed and";
2425
pub const SKILLS_INTRO_WITH_ABSOLUTE_PATHS: &str = "A skill is a set of local instructions to follow that is stored in a `SKILL.md` file. Below is the list of skills that can be used. Each entry includes a name, description, and file path so you can open the source for full instructions when using a specific skill.";
2526
pub const SKILLS_INTRO_WITH_ALIASES: &str = "A skill is a set of local instructions to follow that is stored in a `SKILL.md` file. Below is the list of skills that can be used. Each entry includes a name, description, and a short path that can be expanded into an absolute path using the skill roots table.";
2627
pub const SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS: &str = r###"- Discovery: The list above is the skills available in this session (name + description + file path). Skill bodies live on disk at the listed paths.
@@ -230,11 +231,13 @@ fn build_available_skills_from_lines(
230231
} else if report.average_truncated_description_chars()
231232
> SKILL_DESCRIPTION_TRUNCATION_WARNING_THRESHOLD_CHARS
232233
{
233-
Some(format!(
234-
"{} {} characters per skill.",
235-
budget_warning_prefix(budget, SKILL_DESCRIPTION_TRUNCATED_WARNING_PREFIX),
236-
report.average_truncated_description_chars()
237-
))
234+
Some(
235+
match budget {
236+
SkillMetadataBudget::Tokens(_) => SKILL_DESCRIPTION_TRUNCATED_WARNING_WITH_PERCENT,
237+
SkillMetadataBudget::Characters(_) => SKILL_DESCRIPTION_TRUNCATED_WARNING,
238+
}
239+
.to_string(),
240+
)
238241
} else {
239242
None
240243
};
@@ -431,13 +434,13 @@ fn skill_render_report(
431434

432435
impl SkillRenderReport {
433436
fn average_truncated_description_chars(&self) -> usize {
434-
if self.truncated_description_count == 0 {
437+
if self.total_count == 0 || self.truncated_description_chars == 0 {
435438
return 0;
436439
}
437440

438441
self.truncated_description_chars
439-
.saturating_add(self.truncated_description_count.saturating_sub(1))
440-
/ self.truncated_description_count
442+
.saturating_add(self.total_count.saturating_sub(1))
443+
/ self.total_count
441444
}
442445
}
443446

@@ -1048,30 +1051,51 @@ mod tests {
10481051

10491052
#[test]
10501053
fn budgeted_rendering_warns_when_average_description_truncation_exceeds_threshold() {
1051-
let alpha =
1052-
make_skill_with_description("alpha-skill", SkillScope::Repo, "abcdefghijklmnop");
1053-
let beta = make_skill_with_description("beta-skill", SkillScope::Repo, "uvwxyzabcdefghij");
1054-
let minimum_cost = SkillLine::new(&alpha)
1054+
let long_description = "a".repeat(250);
1055+
let long_skill =
1056+
make_skill_with_description("long-skill", SkillScope::Repo, &long_description);
1057+
let empty_skill = make_skill_with_description("empty-skill", SkillScope::Repo, "");
1058+
let minimum_cost = SkillLine::new(&long_skill)
10551059
.minimum_cost(SkillMetadataBudget::Characters(usize::MAX))
1056-
+ SkillLine::new(&beta).minimum_cost(SkillMetadataBudget::Characters(usize::MAX));
1057-
let budget = SkillMetadataBudget::Characters(minimum_cost + 6);
1060+
+ SkillLine::new(&empty_skill)
1061+
.minimum_cost(SkillMetadataBudget::Characters(usize::MAX));
1062+
let budget = SkillMetadataBudget::Characters(minimum_cost + 49);
10581063

1059-
let rendered = build_available_skills_from_metadata(&[alpha, beta], budget)
1064+
let rendered = build_available_skills_from_metadata(&[long_skill, empty_skill], budget)
10601065
.expect("skills should render");
10611066

1067+
assert_eq!(rendered.report.total_count, 2);
10621068
assert_eq!(rendered.report.included_count, 2);
10631069
assert_eq!(rendered.report.omitted_count, 0);
1064-
assert_eq!(rendered.report.truncated_description_chars, 28);
1065-
assert_eq!(rendered.report.truncated_description_count, 2);
1070+
assert_eq!(rendered.report.truncated_description_chars, 202);
1071+
assert_eq!(rendered.report.truncated_description_count, 1);
10661072
assert_eq!(
10671073
rendered.warning_message,
10681074
Some(
1069-
"Warning: Exceeded skills context budget. Loaded skill descriptions were truncated by an average of 14 characters per skill."
1075+
"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."
10701076
.to_string()
10711077
)
10721078
);
10731079
}
10741080

1081+
#[test]
1082+
fn budgeted_rendering_token_budget_truncation_warning_mentions_two_percent() {
1083+
let long_description = "a".repeat(1000);
1084+
let long_skill =
1085+
make_skill_with_description("long-skill", SkillScope::Repo, &long_description);
1086+
let minimum_cost =
1087+
SkillLine::new(&long_skill).minimum_cost(SkillMetadataBudget::Tokens(usize::MAX));
1088+
let budget = SkillMetadataBudget::Tokens(minimum_cost + 1);
1089+
1090+
let rendered = build_available_skills_from_metadata(&[long_skill], budget)
1091+
.expect("skills should render");
1092+
1093+
assert_eq!(
1094+
rendered.warning_message,
1095+
Some(SKILL_DESCRIPTION_TRUNCATED_WARNING_WITH_PERCENT.to_string())
1096+
);
1097+
}
1098+
10751099
#[test]
10761100
fn budgeted_rendering_redistributes_unused_description_budget() {
10771101
let short = make_skill_with_description("short-skill", SkillScope::Repo, "x");
@@ -1116,7 +1140,7 @@ mod tests {
11161140
assert_eq!(
11171141
rendered.warning_message,
11181142
Some(
1119-
"Warning: Exceeded skills context budget. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list."
1143+
"Exceeded skills context budget. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list."
11201144
.to_string()
11211145
)
11221146
);
@@ -1145,7 +1169,7 @@ mod tests {
11451169
assert_eq!(
11461170
rendered.warning_message,
11471171
Some(
1148-
"Warning: Exceeded skills context budget. All skill descriptions were removed and 1 additional skill was not included in the model-visible skills list."
1172+
"Exceeded skills context budget. All skill descriptions were removed and 1 additional skill was not included in the model-visible skills list."
11491173
.to_string()
11501174
)
11511175
);

codex-rs/core/src/session/tests.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5607,7 +5607,7 @@ fn emit_thread_start_skill_metrics_records_enabled_kept_and_truncated_values() {
56075607
assert_eq!(
56085608
rendered.warning_message,
56095609
Some(
5610-
"Warning: Exceeded skills context budget. All skill descriptions were removed and 1 additional skill was not included in the model-visible skills list."
5610+
"Exceeded skills context budget. All skill descriptions were removed and 1 additional skill was not included in the model-visible skills list."
56115611
.to_string()
56125612
)
56135613
);
@@ -5718,7 +5718,7 @@ async fn build_initial_context_emits_thread_start_skill_warning_on_repeated_buil
57185718
assert!(matches!(
57195719
warning_event.msg,
57205720
EventMsg::Warning(WarningEvent { message })
5721-
if message == "Warning: Exceeded skills context budget of 2%. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list."
5721+
if message == "Exceeded skills context budget of 2%. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list."
57225722
));
57235723

57245724
let _ = session.build_initial_context(&turn_context).await;
@@ -5729,7 +5729,7 @@ async fn build_initial_context_emits_thread_start_skill_warning_on_repeated_buil
57295729
assert!(matches!(
57305730
warning_event.msg,
57315731
EventMsg::Warning(WarningEvent { message })
5732-
if message == "Warning: Exceeded skills context budget of 2%. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list."
5732+
if message == "Exceeded skills context budget of 2%. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list."
57335733
));
57345734
}
57355735

codex-rs/tui/src/chatwidget/tests/app_server.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ async fn live_app_server_warning_notification_renders_message() {
191191
chat.handle_server_notification(
192192
ServerNotification::Warning(WarningNotification {
193193
thread_id: None,
194-
message: "Warning: Exceeded skills context budget of 2%. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list.".to_string(),
194+
message: "Exceeded skills context budget of 2%. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list.".to_string(),
195195
}),
196196
/*replay_kind*/ None,
197197
);
@@ -201,7 +201,7 @@ async fn live_app_server_warning_notification_renders_message() {
201201
let rendered = lines_to_single_string(&cells[0]);
202202
let normalized = rendered.split_whitespace().collect::<Vec<_>>().join(" ");
203203
assert!(
204-
normalized.contains("Warning: Exceeded skills context budget of 2%."),
204+
normalized.contains("Exceeded skills context budget of 2%."),
205205
"expected warning notification message, got {rendered}"
206206
);
207207
assert!(

0 commit comments

Comments
 (0)