Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion codex-rs/app-server/tests/suite/v2/turn_start.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ async fn turn_start_emits_thread_scoped_warning_notification_for_trimmed_skills(
assert_eq!(warning.thread_id.as_deref(), Some(thread.id.as_str()));
assert_eq!(
warning.message,
"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."
"Exceeded skills context budget of 2%. All skill descriptions were removed and 7 additional skills were not included in the model-visible skills list."
);

timeout(
Expand Down
70 changes: 47 additions & 23 deletions codex-rs/core-skills/src/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@ use codex_utils_output_truncation::approx_token_count;

const DEFAULT_SKILL_METADATA_CHAR_BUDGET: usize = 8_000;
const SKILL_METADATA_CONTEXT_WINDOW_PERCENT: usize = 2;
const SKILL_DESCRIPTION_TRUNCATION_WARNING_THRESHOLD_CHARS: usize = 10;
const SKILL_DESCRIPTION_TRUNCATION_WARNING_THRESHOLD_CHARS: usize = 100;
const APPROX_BYTES_PER_TOKEN: usize = 4;
pub const SKILL_DESCRIPTION_TRUNCATED_WARNING_PREFIX: &str = "Warning: Exceeded skills context budget. Loaded skill descriptions were truncated by an average of";
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.";
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.";
pub const SKILL_DESCRIPTIONS_REMOVED_WARNING_PREFIX: &str =
"Warning: Exceeded skills context budget. All skill descriptions were removed and";
"Exceeded skills context budget. All skill descriptions were removed and";
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.";
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.";
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.
Expand Down Expand Up @@ -230,11 +231,13 @@ fn build_available_skills_from_lines(
} else if report.average_truncated_description_chars()
> SKILL_DESCRIPTION_TRUNCATION_WARNING_THRESHOLD_CHARS
{
Some(format!(
"{} {} characters per skill.",
budget_warning_prefix(budget, SKILL_DESCRIPTION_TRUNCATED_WARNING_PREFIX),
report.average_truncated_description_chars()
))
Some(
match budget {
SkillMetadataBudget::Tokens(_) => SKILL_DESCRIPTION_TRUNCATED_WARNING_WITH_PERCENT,
SkillMetadataBudget::Characters(_) => SKILL_DESCRIPTION_TRUNCATED_WARNING,
}
.to_string(),
)
} else {
None
};
Expand Down Expand Up @@ -431,13 +434,13 @@ fn skill_render_report(

impl SkillRenderReport {
fn average_truncated_description_chars(&self) -> usize {
if self.truncated_description_count == 0 {
if self.total_count == 0 || self.truncated_description_chars == 0 {
return 0;
}

self.truncated_description_chars
.saturating_add(self.truncated_description_count.saturating_sub(1))
/ self.truncated_description_count
.saturating_add(self.total_count.saturating_sub(1))
/ self.total_count
}
}

Expand Down Expand Up @@ -1048,30 +1051,51 @@ mod tests {

#[test]
fn budgeted_rendering_warns_when_average_description_truncation_exceeds_threshold() {
let alpha =
make_skill_with_description("alpha-skill", SkillScope::Repo, "abcdefghijklmnop");
let beta = make_skill_with_description("beta-skill", SkillScope::Repo, "uvwxyzabcdefghij");
let minimum_cost = SkillLine::new(&alpha)
let long_description = "a".repeat(250);
let long_skill =
make_skill_with_description("long-skill", SkillScope::Repo, &long_description);
let empty_skill = make_skill_with_description("empty-skill", SkillScope::Repo, "");
let minimum_cost = SkillLine::new(&long_skill)
.minimum_cost(SkillMetadataBudget::Characters(usize::MAX))
+ SkillLine::new(&beta).minimum_cost(SkillMetadataBudget::Characters(usize::MAX));
let budget = SkillMetadataBudget::Characters(minimum_cost + 6);
+ SkillLine::new(&empty_skill)
.minimum_cost(SkillMetadataBudget::Characters(usize::MAX));
let budget = SkillMetadataBudget::Characters(minimum_cost + 49);

let rendered = build_available_skills_from_metadata(&[alpha, beta], budget)
let rendered = build_available_skills_from_metadata(&[long_skill, empty_skill], budget)
.expect("skills should render");

assert_eq!(rendered.report.total_count, 2);
assert_eq!(rendered.report.included_count, 2);
assert_eq!(rendered.report.omitted_count, 0);
assert_eq!(rendered.report.truncated_description_chars, 28);
assert_eq!(rendered.report.truncated_description_count, 2);
assert_eq!(rendered.report.truncated_description_chars, 202);
assert_eq!(rendered.report.truncated_description_count, 1);
assert_eq!(
rendered.warning_message,
Some(
"Warning: Exceeded skills context budget. Loaded skill descriptions were truncated by an average of 14 characters per skill."
"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 budgeted_rendering_token_budget_truncation_warning_mentions_two_percent() {
let long_description = "a".repeat(1000);
let long_skill =
make_skill_with_description("long-skill", SkillScope::Repo, &long_description);
let minimum_cost =
SkillLine::new(&long_skill).minimum_cost(SkillMetadataBudget::Tokens(usize::MAX));
let budget = SkillMetadataBudget::Tokens(minimum_cost + 1);

let rendered = build_available_skills_from_metadata(&[long_skill], budget)
.expect("skills should render");

assert_eq!(
rendered.warning_message,
Some(SKILL_DESCRIPTION_TRUNCATED_WARNING_WITH_PERCENT.to_string())
);
}

#[test]
fn budgeted_rendering_redistributes_unused_description_budget() {
let short = make_skill_with_description("short-skill", SkillScope::Repo, "x");
Expand Down Expand Up @@ -1116,7 +1140,7 @@ mod tests {
assert_eq!(
rendered.warning_message,
Some(
"Warning: Exceeded skills context budget. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list."
"Exceeded skills context budget. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list."
.to_string()
)
);
Expand Down Expand Up @@ -1145,7 +1169,7 @@ mod tests {
assert_eq!(
rendered.warning_message,
Some(
"Warning: Exceeded skills context budget. All skill descriptions were removed and 1 additional skill was not included in the model-visible skills list."
"Exceeded skills context budget. All skill descriptions were removed and 1 additional skill was not included in the model-visible skills list."
.to_string()
)
);
Expand Down
6 changes: 3 additions & 3 deletions codex-rs/core/src/session/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5607,7 +5607,7 @@ fn emit_thread_start_skill_metrics_records_enabled_kept_and_truncated_values() {
assert_eq!(
rendered.warning_message,
Some(
"Warning: Exceeded skills context budget. All skill descriptions were removed and 1 additional skill was not included in the model-visible skills list."
"Exceeded skills context budget. All skill descriptions were removed and 1 additional skill was not included in the model-visible skills list."
.to_string()
)
);
Expand Down Expand Up @@ -5718,7 +5718,7 @@ async fn build_initial_context_emits_thread_start_skill_warning_on_repeated_buil
assert!(matches!(
warning_event.msg,
EventMsg::Warning(WarningEvent { message })
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."
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."
));

let _ = session.build_initial_context(&turn_context).await;
Expand All @@ -5729,7 +5729,7 @@ async fn build_initial_context_emits_thread_start_skill_warning_on_repeated_buil
assert!(matches!(
warning_event.msg,
EventMsg::Warning(WarningEvent { message })
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."
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."
));
}

Expand Down
4 changes: 2 additions & 2 deletions codex-rs/tui/src/chatwidget/tests/app_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ async fn live_app_server_warning_notification_renders_message() {
chat.handle_server_notification(
ServerNotification::Warning(WarningNotification {
thread_id: None,
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(),
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(),
}),
/*replay_kind*/ None,
);
Expand All @@ -201,7 +201,7 @@ async fn live_app_server_warning_notification_renders_message() {
let rendered = lines_to_single_string(&cells[0]);
let normalized = rendered.split_whitespace().collect::<Vec<_>>().join(" ");
assert!(
normalized.contains("Warning: Exceeded skills context budget of 2%."),
normalized.contains("Exceeded skills context budget of 2%."),
"expected warning notification message, got {rendered}"
);
assert!(
Expand Down
Loading