mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Share the skills budget across host and executor catalogs (#35769)
## Why Rendering host and executor skill catalogs independently allows their combined metadata to exceed the skills context budget. ## What changed - Allocate one budget across both catalogs when both contain model-visible skills. - Prefer retaining executor skills when budget pressure requires omissions, while preserving an executor omission marker and reporting when all host skills were omitted. - Evaluate absolute and aliased host paths under the shared budget and select the rendering that best preserves executor entries, total entries, and descriptions, in that order. ## Testing Added renderer and production-turn coverage for shared-budget description shortening, host-first omission, executor omission markers, and alias selection. GitOrigin-RevId: 30041736a76aa173cb641aa98a876a306cb437d2
This commit is contained in:
1 parent
a68d0a74bd
commit
f6160ca5b3
6 files changed
+688
-110
No files matched your search
@@ -42,6 +42,7 @@ use crate::fragments::SkillInstructions;
|
||||
use crate::provider::HostSkillProvider;
|
||||
use crate::provider::SkillListQuery;
|
||||
use crate::provider::SkillReadRequest;
|
||||
use crate::render::AvailableSkillsRender;
|
||||
use crate::render::MAX_SKILL_NAME_BYTES;
|
||||
use crate::render::MAX_SKILL_PATH_BYTES;
|
||||
use crate::render::SkillCatalogRenderPolicy;
|
||||
@@ -49,10 +50,12 @@ use crate::render::SkillMetadataBudget;
|
||||
use crate::render::SkillRenderReport;
|
||||
use crate::render::capped_skill_metadata_budget;
|
||||
use crate::render::render_available_skills;
|
||||
use crate::render::render_combined_available_skills;
|
||||
use crate::render::truncate_main_prompt_contents;
|
||||
use crate::render::truncate_utf8_to_bytes;
|
||||
use crate::render_observability::CatalogSurface;
|
||||
use crate::render_observability::record_catalog_render;
|
||||
use crate::render_observability::trace_catalog_budget_pressure;
|
||||
use crate::selection::collect_explicit_skill_mentions;
|
||||
use crate::shadow_selection_experiment::ShadowSelectionExperiment;
|
||||
use crate::sources::SkillProviders;
|
||||
@@ -66,6 +69,7 @@ use crate::warnings::bounded_warnings;
|
||||
use crate::world_state::HostSkillsWarningEmitter;
|
||||
use crate::world_state::executor_skills_world_state_section;
|
||||
use crate::world_state::host_skills_world_state_section;
|
||||
use crate::world_state::rendered_host_skills_world_state_section;
|
||||
|
||||
struct SkillsExtension<C> {
|
||||
providers: SkillProviders,
|
||||
@@ -88,7 +92,23 @@ fn render_catalog(
|
||||
policy: SkillCatalogRenderPolicy,
|
||||
budget: SkillMetadataBudget,
|
||||
) -> RenderedCatalog {
|
||||
let Some(rendered) = render_available_skills(catalog, policy, budget) else {
|
||||
render_prepared_catalog(
|
||||
extension_metrics,
|
||||
catalog_surface,
|
||||
include_skills_usage_instructions,
|
||||
budget,
|
||||
render_available_skills(catalog, policy, budget),
|
||||
)
|
||||
}
|
||||
|
||||
fn render_prepared_catalog(
|
||||
extension_metrics: Option<&dyn ExtensionMetrics>,
|
||||
catalog_surface: CatalogSurface,
|
||||
include_skills_usage_instructions: bool,
|
||||
budget: SkillMetadataBudget,
|
||||
rendered: Option<AvailableSkillsRender>,
|
||||
) -> RenderedCatalog {
|
||||
let Some(rendered) = rendered else {
|
||||
record_catalog_render(
|
||||
extension_metrics,
|
||||
catalog_surface,
|
||||
@@ -259,19 +279,78 @@ where
|
||||
.and_then(ModelInfo::resolved_context_window);
|
||||
let metadata_budget = capped_skill_metadata_budget(context_window);
|
||||
let extension_metrics = input.extension_metrics.clone();
|
||||
let rendered = if config.include_instructions {
|
||||
render_catalog(
|
||||
extension_metrics.as_deref(),
|
||||
CatalogSurface::ExecutorWorldState,
|
||||
&catalog,
|
||||
include_usage,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
metadata_budget,
|
||||
)
|
||||
let host_snapshot = input.turn_store.get::<HostSkillsSnapshot>();
|
||||
let shared_rendered = if config.include_instructions
|
||||
&& let Some(host_snapshot) = host_snapshot.as_ref()
|
||||
&& self.providers.has_host_provider()
|
||||
{
|
||||
let host_catalog = self
|
||||
.providers
|
||||
.list_host_for_turn(SkillListQuery {
|
||||
turn_id: input.turn_id.to_string(),
|
||||
executor_roots: Vec::new(),
|
||||
resolved_executor_roots: Vec::new(),
|
||||
host_snapshot: Some(Arc::clone(host_snapshot)),
|
||||
include_host_skills: true,
|
||||
include_bundled_skills: false,
|
||||
include_orchestrator_skills: false,
|
||||
mcp_resources: None,
|
||||
executor_capability_discovery: None,
|
||||
})
|
||||
.await;
|
||||
let (host_rendered, executor_rendered) =
|
||||
render_combined_available_skills(&host_catalog, &catalog, metadata_budget);
|
||||
if host_catalog
|
||||
.entries
|
||||
.iter()
|
||||
.any(SkillCatalogEntry::is_model_visible)
|
||||
&& catalog
|
||||
.entries
|
||||
.iter()
|
||||
.any(SkillCatalogEntry::is_model_visible)
|
||||
{
|
||||
Some((host_rendered, executor_rendered))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
} else {
|
||||
RenderedCatalog::default()
|
||||
None
|
||||
};
|
||||
let warning_message = rendered.warning_message;
|
||||
let (rendered, rendered_host) =
|
||||
if let Some((host_rendered, executor_rendered)) = shared_rendered {
|
||||
if let Some(host_rendered) = host_rendered.as_ref() {
|
||||
trace_catalog_budget_pressure(metadata_budget, &host_rendered.report);
|
||||
}
|
||||
(
|
||||
render_prepared_catalog(
|
||||
extension_metrics.as_deref(),
|
||||
CatalogSurface::ExecutorWorldState,
|
||||
include_usage,
|
||||
metadata_budget,
|
||||
executor_rendered,
|
||||
),
|
||||
host_rendered,
|
||||
)
|
||||
} else {
|
||||
(
|
||||
if config.include_instructions {
|
||||
render_catalog(
|
||||
extension_metrics.as_deref(),
|
||||
CatalogSurface::ExecutorWorldState,
|
||||
&catalog,
|
||||
include_usage,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
metadata_budget,
|
||||
)
|
||||
} else {
|
||||
RenderedCatalog::default()
|
||||
},
|
||||
None,
|
||||
)
|
||||
};
|
||||
let host_warning_message = rendered_host
|
||||
.as_ref()
|
||||
.and_then(|rendered_host| rendered_host.report.warning_message());
|
||||
let executor_body = rendered.fragment.map(|fragment| fragment.body());
|
||||
let mut sections = vec![executor_skills_world_state_section(
|
||||
executor_body,
|
||||
@@ -280,7 +359,7 @@ where
|
||||
let emitted_warnings = input
|
||||
.turn_store
|
||||
.get_or_init(EmittedCatalogBudgetWarnings::default);
|
||||
if let Some(host_snapshot) = input.turn_store.get::<HostSkillsSnapshot>()
|
||||
if let Some(host_snapshot) = host_snapshot
|
||||
&& self.providers.has_host_provider()
|
||||
{
|
||||
input.turn_store.insert(HostSkillsCatalogInWorldState);
|
||||
@@ -297,16 +376,37 @@ where
|
||||
});
|
||||
}
|
||||
});
|
||||
sections.push(host_skills_world_state_section(
|
||||
&host_snapshot,
|
||||
config.include_instructions,
|
||||
include_usage,
|
||||
metadata_budget,
|
||||
extension_metrics,
|
||||
warning_emitter,
|
||||
));
|
||||
if let Some(rendered_host) = rendered_host {
|
||||
let report = &rendered_host.report;
|
||||
let render_metrics = Some((
|
||||
report.total_count,
|
||||
report.included_count,
|
||||
report.omitted_count,
|
||||
report.truncated_description_chars,
|
||||
));
|
||||
let body = rendered_host
|
||||
.into_fragment(include_usage)
|
||||
.map(|fragment| fragment.body());
|
||||
sections.push(rendered_host_skills_world_state_section(
|
||||
body,
|
||||
config.include_instructions,
|
||||
render_metrics,
|
||||
extension_metrics,
|
||||
host_warning_message,
|
||||
warning_emitter,
|
||||
));
|
||||
} else {
|
||||
sections.push(host_skills_world_state_section(
|
||||
&host_snapshot,
|
||||
config.include_instructions,
|
||||
include_usage,
|
||||
metadata_budget,
|
||||
extension_metrics,
|
||||
warning_emitter,
|
||||
));
|
||||
}
|
||||
}
|
||||
if let Some(message) = warning_message
|
||||
if let Some(message) = rendered.warning_message
|
||||
&& emitted_warnings.insert(&message)
|
||||
{
|
||||
self.emit_warning(input.thread_store.level_id(), Some(input.turn_id), message);
|
||||
|
||||
@@ -31,13 +31,6 @@ pub(crate) const MAX_SKILL_PATH_BYTES: usize = 1_024;
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub(crate) enum SkillCatalogRenderPolicy {
|
||||
#[cfg_attr(
|
||||
not(test),
|
||||
expect(
|
||||
dead_code,
|
||||
reason = "used by the host renderer compatibility path in a follow-up"
|
||||
)
|
||||
)]
|
||||
CoreCompatible,
|
||||
ExtensionCompatible,
|
||||
}
|
||||
@@ -256,7 +249,6 @@ impl<'a> SkillLine<'a> {
|
||||
|
||||
struct RenderedSkillLine {
|
||||
line: String,
|
||||
truncated_description_chars: usize,
|
||||
}
|
||||
|
||||
struct RenderedSkillLines {
|
||||
@@ -266,14 +258,13 @@ struct RenderedSkillLines {
|
||||
truncated_description_count: usize,
|
||||
}
|
||||
|
||||
struct DescriptionBudgetLine<'a> {
|
||||
line: &'a SkillLine<'a>,
|
||||
struct DescriptionBudgetLine {
|
||||
description_char_count: usize,
|
||||
extra_costs: Vec<usize>,
|
||||
}
|
||||
|
||||
impl<'a> DescriptionBudgetLine<'a> {
|
||||
fn new(line: &'a SkillLine<'a>, budget: SkillMetadataBudget) -> Self {
|
||||
impl DescriptionBudgetLine {
|
||||
fn new(line: &SkillLine<'_>, budget: SkillMetadataBudget) -> Self {
|
||||
let minimum_line = line.render_minimum();
|
||||
let minimum_chars = minimum_line.chars().count().saturating_add(1);
|
||||
let minimum_bytes = minimum_line.len().saturating_add(1);
|
||||
@@ -297,92 +288,114 @@ impl<'a> DescriptionBudgetLine<'a> {
|
||||
}
|
||||
|
||||
Self {
|
||||
line,
|
||||
description_char_count,
|
||||
extra_costs,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
enum SkillLineAllocation {
|
||||
Omitted,
|
||||
DescriptionChars(usize),
|
||||
}
|
||||
|
||||
fn render_skill_lines(
|
||||
skill_lines: Vec<SkillLine<'_>>,
|
||||
budget: SkillMetadataBudget,
|
||||
) -> RenderedSkillLines {
|
||||
let allocations = allocate_skill_lines(&skill_lines, budget);
|
||||
render_allocated_skill_lines(&skill_lines, &allocations)
|
||||
}
|
||||
|
||||
fn allocate_skill_lines(
|
||||
skill_lines: &[SkillLine<'_>],
|
||||
budget: SkillMetadataBudget,
|
||||
) -> Vec<SkillLineAllocation> {
|
||||
let full_cost = skill_lines.iter().fold(0usize, |used, line| {
|
||||
used.saturating_add(line.full_cost(budget))
|
||||
});
|
||||
if full_cost <= budget.limit() {
|
||||
return RenderedSkillLines {
|
||||
lines: skill_lines
|
||||
.iter()
|
||||
.map(|line| RenderedSkillLine {
|
||||
line: line.render_full(),
|
||||
truncated_description_chars: 0,
|
||||
})
|
||||
.collect(),
|
||||
omitted_count: 0,
|
||||
truncated_description_chars: 0,
|
||||
truncated_description_count: 0,
|
||||
};
|
||||
return skill_lines
|
||||
.iter()
|
||||
.map(|line| SkillLineAllocation::DescriptionChars(line.description_char_count()))
|
||||
.collect();
|
||||
}
|
||||
|
||||
let minimum_cost = skill_lines.iter().fold(0usize, |used, line| {
|
||||
used.saturating_add(line.minimum_cost(budget))
|
||||
});
|
||||
if minimum_cost <= budget.limit() {
|
||||
let lines = render_lines_with_description_budget(
|
||||
return allocate_description_chars(
|
||||
budget,
|
||||
&skill_lines,
|
||||
skill_lines,
|
||||
budget.limit().saturating_sub(minimum_cost),
|
||||
);
|
||||
let (truncated_description_chars, truncated_description_count) =
|
||||
sum_description_truncation(&lines);
|
||||
return RenderedSkillLines {
|
||||
lines,
|
||||
omitted_count: 0,
|
||||
truncated_description_chars,
|
||||
truncated_description_count,
|
||||
};
|
||||
)
|
||||
.into_iter()
|
||||
.map(SkillLineAllocation::DescriptionChars)
|
||||
.collect();
|
||||
}
|
||||
|
||||
let mut included = Vec::new();
|
||||
let mut used = 0usize;
|
||||
let mut omitted = 0usize;
|
||||
skill_lines
|
||||
.iter()
|
||||
.map(|line| {
|
||||
let next_used = used.saturating_add(line.minimum_cost(budget));
|
||||
if next_used <= budget.limit() {
|
||||
used = next_used;
|
||||
SkillLineAllocation::DescriptionChars(0)
|
||||
} else {
|
||||
SkillLineAllocation::Omitted
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn render_allocated_skill_lines(
|
||||
skill_lines: &[SkillLine<'_>],
|
||||
allocations: &[SkillLineAllocation],
|
||||
) -> RenderedSkillLines {
|
||||
let mut lines = Vec::new();
|
||||
let mut omitted_count = 0usize;
|
||||
let mut truncated_description_chars = 0usize;
|
||||
let mut truncated_description_count = 0usize;
|
||||
for line in skill_lines {
|
||||
for (line, allocation) in skill_lines.iter().zip(allocations) {
|
||||
let description_char_count = line.description_char_count();
|
||||
let rendered = line.render_minimum();
|
||||
let next_used = used.saturating_add(line.minimum_cost(budget));
|
||||
if next_used <= budget.limit() {
|
||||
used = next_used;
|
||||
included.push(RenderedSkillLine {
|
||||
line: rendered,
|
||||
truncated_description_chars: description_char_count,
|
||||
});
|
||||
} else {
|
||||
omitted = omitted.saturating_add(1);
|
||||
}
|
||||
|
||||
truncated_description_chars =
|
||||
truncated_description_chars.saturating_add(description_char_count);
|
||||
if description_char_count > 0 {
|
||||
truncated_description_count = truncated_description_count.saturating_add(1);
|
||||
match allocation {
|
||||
SkillLineAllocation::Omitted => {
|
||||
omitted_count = omitted_count.saturating_add(1);
|
||||
truncated_description_chars =
|
||||
truncated_description_chars.saturating_add(description_char_count);
|
||||
if description_char_count > 0 {
|
||||
truncated_description_count = truncated_description_count.saturating_add(1);
|
||||
}
|
||||
}
|
||||
SkillLineAllocation::DescriptionChars(description_chars) => {
|
||||
let truncated_chars = description_char_count.saturating_sub(*description_chars);
|
||||
if truncated_chars > 0 {
|
||||
truncated_description_chars =
|
||||
truncated_description_chars.saturating_add(truncated_chars);
|
||||
truncated_description_count = truncated_description_count.saturating_add(1);
|
||||
}
|
||||
lines.push(RenderedSkillLine {
|
||||
line: line.render_with_description_chars(*description_chars),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
RenderedSkillLines {
|
||||
lines: included,
|
||||
omitted_count: omitted,
|
||||
lines,
|
||||
omitted_count,
|
||||
truncated_description_chars,
|
||||
truncated_description_count,
|
||||
}
|
||||
}
|
||||
|
||||
fn render_lines_with_description_budget(
|
||||
fn allocate_description_chars(
|
||||
budget: SkillMetadataBudget,
|
||||
skill_lines: &[SkillLine<'_>],
|
||||
limit: usize,
|
||||
) -> Vec<RenderedSkillLine> {
|
||||
) -> Vec<usize> {
|
||||
let budget_lines = skill_lines
|
||||
.iter()
|
||||
.map(|line| DescriptionBudgetLine::new(line, budget))
|
||||
@@ -416,31 +429,7 @@ fn render_lines_with_description_budget(
|
||||
}
|
||||
}
|
||||
|
||||
budget_lines
|
||||
.iter()
|
||||
.zip(char_allocations)
|
||||
.map(|(line, description_chars)| RenderedSkillLine {
|
||||
line: line.line.render_with_description_chars(description_chars),
|
||||
truncated_description_chars: line
|
||||
.description_char_count
|
||||
.saturating_sub(description_chars),
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn sum_description_truncation(rendered: &[RenderedSkillLine]) -> (usize, usize) {
|
||||
rendered
|
||||
.iter()
|
||||
.fold((0usize, 0usize), |(chars, count), line| {
|
||||
if line.truncated_description_chars == 0 {
|
||||
(chars, count)
|
||||
} else {
|
||||
(
|
||||
chars.saturating_add(line.truncated_description_chars),
|
||||
count.saturating_add(1),
|
||||
)
|
||||
}
|
||||
})
|
||||
char_allocations
|
||||
}
|
||||
|
||||
struct RenderedCatalog {
|
||||
@@ -517,6 +506,279 @@ pub(crate) fn render_available_skills(
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn render_combined_available_skills(
|
||||
host_catalog: &SkillCatalog,
|
||||
executor_catalog: &SkillCatalog,
|
||||
budget: SkillMetadataBudget,
|
||||
) -> (Option<AvailableSkillsRender>, Option<AvailableSkillsRender>) {
|
||||
let mut host_entries = host_catalog
|
||||
.entries
|
||||
.iter()
|
||||
.filter(|entry| entry.is_model_visible())
|
||||
.collect::<Vec<_>>();
|
||||
let mut executor_entries = executor_catalog
|
||||
.entries
|
||||
.iter()
|
||||
.filter(|entry| entry.is_model_visible())
|
||||
.collect::<Vec<_>>();
|
||||
SkillCatalogRenderPolicy::CoreCompatible.order_entries(&mut host_entries);
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible.order_entries(&mut executor_entries);
|
||||
if host_entries.is_empty() || executor_entries.is_empty() {
|
||||
return (
|
||||
render_available_skills(
|
||||
host_catalog,
|
||||
SkillCatalogRenderPolicy::CoreCompatible,
|
||||
budget,
|
||||
),
|
||||
render_available_skills(
|
||||
executor_catalog,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
budget,
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
let absolute = render_combined_lines(
|
||||
host_entries
|
||||
.iter()
|
||||
.map(|entry| SkillLine::new(entry, SkillCatalogRenderPolicy::CoreCompatible))
|
||||
.collect(),
|
||||
executor_entries
|
||||
.iter()
|
||||
.map(|entry| SkillLine::new(entry, SkillCatalogRenderPolicy::ExtensionCompatible))
|
||||
.collect(),
|
||||
budget,
|
||||
Vec::new(),
|
||||
);
|
||||
let selected = if combined_catalog_fully_rendered(&absolute) {
|
||||
absolute
|
||||
} else if let Some(aliased) =
|
||||
build_aliased_combined_catalog(&host_entries, &executor_entries, budget)
|
||||
&& combined_render_is_better(&aliased, &absolute, budget)
|
||||
{
|
||||
aliased
|
||||
} else {
|
||||
absolute
|
||||
};
|
||||
|
||||
(Some(selected.host), Some(selected.executor))
|
||||
}
|
||||
|
||||
struct CombinedAvailableSkillsRender {
|
||||
host: AvailableSkillsRender,
|
||||
executor: AvailableSkillsRender,
|
||||
}
|
||||
|
||||
fn render_combined_lines(
|
||||
host_lines: Vec<SkillLine<'_>>,
|
||||
executor_lines: Vec<SkillLine<'_>>,
|
||||
budget: SkillMetadataBudget,
|
||||
host_skill_root_lines: Vec<String>,
|
||||
) -> CombinedAvailableSkillsRender {
|
||||
let executor_count = executor_lines.len();
|
||||
let mut lines = executor_lines;
|
||||
lines.extend(host_lines);
|
||||
let mut allocations = allocate_skill_lines(&lines, budget);
|
||||
let executor_omission_marker =
|
||||
reserve_executor_omission_marker(&lines, executor_count, budget, &mut allocations);
|
||||
|
||||
CombinedAvailableSkillsRender {
|
||||
executor: render_combined_group(
|
||||
&lines[..executor_count],
|
||||
&allocations[..executor_count],
|
||||
Vec::new(),
|
||||
executor_omission_marker,
|
||||
),
|
||||
host: render_combined_group(
|
||||
&lines[executor_count..],
|
||||
&allocations[executor_count..],
|
||||
host_skill_root_lines,
|
||||
/*omission_marker*/ None,
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
fn render_combined_group(
|
||||
skill_lines: &[SkillLine<'_>],
|
||||
allocations: &[SkillLineAllocation],
|
||||
skill_root_lines: Vec<String>,
|
||||
omission_marker: Option<String>,
|
||||
) -> AvailableSkillsRender {
|
||||
let RenderedSkillLines {
|
||||
mut lines,
|
||||
omitted_count,
|
||||
truncated_description_chars,
|
||||
truncated_description_count,
|
||||
} = render_allocated_skill_lines(skill_lines, allocations);
|
||||
if let Some(marker) = omission_marker {
|
||||
lines.push(RenderedSkillLine { line: marker });
|
||||
}
|
||||
AvailableSkillsRender {
|
||||
skill_root_lines,
|
||||
skill_lines: lines.into_iter().map(|rendered| rendered.line).collect(),
|
||||
report: SkillRenderReport {
|
||||
total_count: skill_lines.len(),
|
||||
included_count: skill_lines.len().saturating_sub(omitted_count),
|
||||
omitted_count,
|
||||
truncated_description_chars,
|
||||
truncated_description_count,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
fn build_aliased_combined_catalog(
|
||||
host_entries: &[&SkillCatalogEntry],
|
||||
executor_entries: &[&SkillCatalogEntry],
|
||||
budget: SkillMetadataBudget,
|
||||
) -> Option<CombinedAvailableSkillsRender> {
|
||||
let plan = build_alias_plan(host_entries, budget)?;
|
||||
if plan.table_cost >= budget.limit() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let adjusted_limit = budget.limit().saturating_sub(plan.table_cost);
|
||||
let adjusted_budget = match budget {
|
||||
SkillMetadataBudget::Tokens(_) => SkillMetadataBudget::Tokens(adjusted_limit),
|
||||
SkillMetadataBudget::Characters(_) => SkillMetadataBudget::Characters(adjusted_limit),
|
||||
};
|
||||
let host_lines = host_entries
|
||||
.iter()
|
||||
.map(|entry| {
|
||||
SkillLine::with_locator(
|
||||
entry,
|
||||
SkillCatalogRenderPolicy::CoreCompatible,
|
||||
render_skill_path_with_aliases(entry, &plan),
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
let executor_lines = executor_entries
|
||||
.iter()
|
||||
.map(|entry| SkillLine::new(entry, SkillCatalogRenderPolicy::ExtensionCompatible))
|
||||
.collect();
|
||||
Some(render_combined_lines(
|
||||
host_lines,
|
||||
executor_lines,
|
||||
adjusted_budget,
|
||||
plan.skill_root_lines,
|
||||
))
|
||||
}
|
||||
|
||||
fn combined_catalog_fully_rendered(rendered: &CombinedAvailableSkillsRender) -> bool {
|
||||
rendered.host.report.omitted_count == 0
|
||||
&& rendered.host.report.truncated_description_chars == 0
|
||||
&& rendered.executor.report.omitted_count == 0
|
||||
&& rendered.executor.report.truncated_description_chars == 0
|
||||
}
|
||||
|
||||
fn combined_render_is_better(
|
||||
candidate: &CombinedAvailableSkillsRender,
|
||||
current: &CombinedAvailableSkillsRender,
|
||||
budget: SkillMetadataBudget,
|
||||
) -> bool {
|
||||
if candidate.executor.report.included_count != current.executor.report.included_count {
|
||||
return candidate.executor.report.included_count > current.executor.report.included_count;
|
||||
}
|
||||
|
||||
let included_count = |rendered: &CombinedAvailableSkillsRender| {
|
||||
rendered
|
||||
.host
|
||||
.report
|
||||
.included_count
|
||||
.saturating_add(rendered.executor.report.included_count)
|
||||
};
|
||||
if included_count(candidate) != included_count(current) {
|
||||
return included_count(candidate) > included_count(current);
|
||||
}
|
||||
|
||||
let truncated_chars = |rendered: &CombinedAvailableSkillsRender| {
|
||||
rendered
|
||||
.host
|
||||
.report
|
||||
.truncated_description_chars
|
||||
.saturating_add(rendered.executor.report.truncated_description_chars)
|
||||
};
|
||||
if truncated_chars(candidate) != truncated_chars(current) {
|
||||
return truncated_chars(candidate) < truncated_chars(current);
|
||||
}
|
||||
|
||||
combined_available_skills_cost(budget, candidate)
|
||||
< combined_available_skills_cost(budget, current)
|
||||
}
|
||||
|
||||
fn combined_available_skills_cost(
|
||||
budget: SkillMetadataBudget,
|
||||
rendered: &CombinedAvailableSkillsRender,
|
||||
) -> usize {
|
||||
[&rendered.host, &rendered.executor]
|
||||
.into_iter()
|
||||
.fold(0usize, |used, catalog| {
|
||||
let root_cost = if !catalog.skill_root_lines.is_empty() {
|
||||
aliased_metadata_overhead_cost(budget, &catalog.skill_root_lines)
|
||||
} else {
|
||||
Default::default()
|
||||
};
|
||||
catalog
|
||||
.skill_lines
|
||||
.iter()
|
||||
.fold(used.saturating_add(root_cost), |used, line| {
|
||||
used.saturating_add(metadata_line_cost(budget, line))
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
fn reserve_executor_omission_marker(
|
||||
skill_lines: &[SkillLine<'_>],
|
||||
executor_count: usize,
|
||||
budget: SkillMetadataBudget,
|
||||
allocations: &mut [SkillLineAllocation],
|
||||
) -> Option<String> {
|
||||
loop {
|
||||
let omitted_count = allocations[..executor_count]
|
||||
.iter()
|
||||
.filter(|allocation| matches!(allocation, SkillLineAllocation::Omitted))
|
||||
.count();
|
||||
if omitted_count == 0 {
|
||||
return None;
|
||||
}
|
||||
|
||||
let marker = omission_marker(omitted_count);
|
||||
let used = allocated_skill_lines_cost(skill_lines, allocations, budget);
|
||||
if used.saturating_add(metadata_line_cost(budget, &marker)) <= budget.limit() {
|
||||
return Some(marker);
|
||||
}
|
||||
|
||||
let index = (executor_count..allocations.len())
|
||||
.rev()
|
||||
.chain((0..executor_count).rev())
|
||||
.find(|index| {
|
||||
matches!(
|
||||
allocations[*index],
|
||||
SkillLineAllocation::DescriptionChars(_)
|
||||
)
|
||||
})?;
|
||||
allocations[index] = SkillLineAllocation::Omitted;
|
||||
}
|
||||
}
|
||||
|
||||
fn allocated_skill_lines_cost(
|
||||
skill_lines: &[SkillLine<'_>],
|
||||
allocations: &[SkillLineAllocation],
|
||||
budget: SkillMetadataBudget,
|
||||
) -> usize {
|
||||
skill_lines
|
||||
.iter()
|
||||
.zip(allocations)
|
||||
.fold(0usize, |used, (line, allocation)| match allocation {
|
||||
SkillLineAllocation::Omitted => used,
|
||||
SkillLineAllocation::DescriptionChars(description_chars) => {
|
||||
used.saturating_add(metadata_line_cost(
|
||||
budget,
|
||||
&line.render_with_description_chars(*description_chars),
|
||||
))
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
fn render_catalog(
|
||||
skill_lines: Vec<SkillLine<'_>>,
|
||||
budget: SkillMetadataBudget,
|
||||
@@ -538,10 +800,7 @@ fn render_catalog(
|
||||
loop {
|
||||
let marker = omission_marker(omitted);
|
||||
if total_cost.saturating_add(metadata_line_cost(budget, &marker)) <= budget.limit() {
|
||||
rendered_lines.push(RenderedSkillLine {
|
||||
line: marker,
|
||||
truncated_description_chars: 0,
|
||||
});
|
||||
rendered_lines.push(RenderedSkillLine { line: marker });
|
||||
break;
|
||||
}
|
||||
let Some(rendered) = rendered_lines.pop() else {
|
||||
|
||||
@@ -41,6 +41,13 @@ pub(crate) fn record_catalog_render(
|
||||
report.truncated_description_chars,
|
||||
);
|
||||
|
||||
trace_catalog_budget_pressure(budget, report);
|
||||
}
|
||||
|
||||
pub(crate) fn trace_catalog_budget_pressure(
|
||||
budget: SkillMetadataBudget,
|
||||
report: &SkillRenderReport,
|
||||
) {
|
||||
if report.omitted_count > 0 || report.truncated_description_chars > 0 {
|
||||
tracing::info!(
|
||||
budget_limit = budget.limit(),
|
||||
|
||||
@@ -323,6 +323,135 @@ fn mixed_catalogs_keep_absolute_authority_aware_rendering_under_budget_pressure(
|
||||
assert!(!body.contains("additional skills omitted"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mixed_catalog_reserves_executor_omission_marker_by_omitting_host_first() {
|
||||
let host_catalog = SkillCatalog {
|
||||
entries: vec![entry_with_path(
|
||||
"h", "", /*short_description*/ None, "/h",
|
||||
)],
|
||||
warnings: Vec::new(),
|
||||
};
|
||||
let executor_entry = |name: &str, resource: &str| {
|
||||
SkillCatalogEntry::new(
|
||||
SkillPackageId(name.to_string()),
|
||||
SkillAuthority::new(SkillSourceKind::Executor, "env-1"),
|
||||
name,
|
||||
"",
|
||||
SkillResourceId::new(resource),
|
||||
)
|
||||
.with_display_path(resource)
|
||||
};
|
||||
let executor_catalog = SkillCatalog {
|
||||
entries: vec![
|
||||
executor_entry("e1", "skill://executor/one"),
|
||||
executor_entry(
|
||||
"e2",
|
||||
"skill://executor/this-resource-is-intentionally-too-long",
|
||||
),
|
||||
],
|
||||
warnings: Vec::new(),
|
||||
};
|
||||
|
||||
let (host, executor) = render_combined_available_skills(
|
||||
&host_catalog,
|
||||
&executor_catalog,
|
||||
SkillMetadataBudget::Tokens(28),
|
||||
);
|
||||
let host = host.expect("host catalog should render");
|
||||
let executor = executor.expect("executor catalog should render");
|
||||
|
||||
assert_eq!(
|
||||
host.report,
|
||||
SkillRenderReport {
|
||||
total_count: 1,
|
||||
included_count: 0,
|
||||
omitted_count: 1,
|
||||
truncated_description_chars: 0,
|
||||
truncated_description_count: 0,
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
executor.report,
|
||||
SkillRenderReport {
|
||||
total_count: 2,
|
||||
included_count: 1,
|
||||
omitted_count: 1,
|
||||
truncated_description_chars: 0,
|
||||
truncated_description_count: 0,
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
executor.skill_lines,
|
||||
vec![
|
||||
"- e1: (environment resource: skill://executor/one)".to_string(),
|
||||
"- 1 additional skill omitted from this bounded skills list.".to_string(),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mixed_catalog_prefers_executor_inclusion_over_total_aliased_inclusion() {
|
||||
let root = format!("/{}", "r".repeat(219));
|
||||
let host_catalog = SkillCatalog {
|
||||
entries: ["h1", "h2"]
|
||||
.into_iter()
|
||||
.map(|name| {
|
||||
entry(name, "", /*short_description*/ None)
|
||||
.with_display_path(format!("{root}/{name}/SKILL.md"))
|
||||
.with_display_path_root(root.as_str())
|
||||
})
|
||||
.collect(),
|
||||
warnings: Vec::new(),
|
||||
};
|
||||
let executor_entry = |name: &str, resource: &str| {
|
||||
SkillCatalogEntry::new(
|
||||
SkillPackageId(name.to_string()),
|
||||
SkillAuthority::new(SkillSourceKind::Executor, "env-1"),
|
||||
name,
|
||||
"",
|
||||
SkillResourceId::new(resource),
|
||||
)
|
||||
.with_display_path(resource)
|
||||
};
|
||||
let executor_catalog = SkillCatalog {
|
||||
entries: vec![
|
||||
executor_entry("e1", "skill://executor/one"),
|
||||
executor_entry("e2", &format!("skill://{}", "e".repeat(132))),
|
||||
],
|
||||
warnings: Vec::new(),
|
||||
};
|
||||
|
||||
let (host, executor) = render_combined_available_skills(
|
||||
&host_catalog,
|
||||
&executor_catalog,
|
||||
SkillMetadataBudget::Tokens(74),
|
||||
);
|
||||
let host = host.expect("host catalog should render");
|
||||
let executor = executor.expect("executor catalog should render");
|
||||
|
||||
assert_eq!(
|
||||
executor.report,
|
||||
SkillRenderReport {
|
||||
total_count: 2,
|
||||
included_count: 2,
|
||||
omitted_count: 0,
|
||||
truncated_description_chars: 0,
|
||||
truncated_description_count: 0,
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
host.report,
|
||||
SkillRenderReport {
|
||||
total_count: 2,
|
||||
included_count: 0,
|
||||
omitted_count: 2,
|
||||
truncated_description_chars: 0,
|
||||
truncated_description_count: 0,
|
||||
}
|
||||
);
|
||||
assert_eq!(host.skill_root_lines, Vec::<String>::new());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn singleton_plugin_versions_share_the_marketplace_alias_root() {
|
||||
let github_root = "/Users/test/.codex/plugins/cache/openai-curated/github/hash123/skills";
|
||||
|
||||
@@ -25,6 +25,7 @@ const HIDDEN_EXECUTOR_SKILLS_BODY: &str = "\n## Skills update\nSelected-environm
|
||||
const NO_HOST_SKILLS_BODY: &str =
|
||||
"\n## Host skills update\nNo host skills are currently available.\n";
|
||||
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";
|
||||
const OMITTED_HOST_SKILLS_BODY: &str = "\n## Host skills update\nHost skills are available but omitted from the model-visible skills list because the skills context budget was exceeded.\n";
|
||||
|
||||
pub(crate) type HostSkillsWarningEmitter = Arc<dyn Fn(String) + Send + Sync>;
|
||||
|
||||
@@ -142,6 +143,13 @@ pub(crate) fn rendered_host_skills_world_state_section(
|
||||
warning_message: Option<String>,
|
||||
warning_emitter: HostSkillsWarningEmitter,
|
||||
) -> WorldStateSectionContribution {
|
||||
let body = body.or_else(|| {
|
||||
render_metrics
|
||||
.is_some_and(|(total_count, included_count, omitted_count, _)| {
|
||||
total_count > 0 && included_count == 0 && omitted_count > 0
|
||||
})
|
||||
.then(|| OMITTED_HOST_SKILLS_BODY.to_string())
|
||||
});
|
||||
let snapshot = json!({
|
||||
"body": body,
|
||||
"includeInstructions": include_instructions,
|
||||
|
||||
Reference in new issue
Block a user