mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Add metrics for extension-rendered skill catalogs (#35597)
## What changed - Add an optional `ExtensionMetrics` capability to thread, turn-input, and world-state contributors, backed by session telemetry in the host. - Record skill counts, omissions, and truncated description characters for extension-rendered catalogs, tagged by the catalog surface. - Preserve host session attribution on extension metrics, including the effective model for each turn, and prevent extension tags from overriding host metadata. - Avoid duplicate host-catalog samples by recording world-state metrics only when the section is published or changes. ## Testing - Cover metric values and surface tags, session metadata preservation, turn-level model changes, and host world-state publication behavior. GitOrigin-RevId: 9059e521943a8f81df25a38b03692eddbc1d990d
This commit is contained in:
1 parent
3d519bf63a
commit
d6ea5991e7
23 files changed
+625
-8
No files matched your search
@@ -11,6 +11,7 @@ use codex_extension_api::ContextualUserFragment;
|
||||
use codex_extension_api::ExtensionData;
|
||||
use codex_extension_api::ExtensionEventSink;
|
||||
use codex_extension_api::ExtensionFuture;
|
||||
use codex_extension_api::ExtensionMetrics;
|
||||
use codex_extension_api::ExtensionRegistryBuilder;
|
||||
use codex_extension_api::ExtensionWarning;
|
||||
use codex_extension_api::PromptFragment;
|
||||
@@ -49,6 +50,8 @@ 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::render_observability::CatalogSurface;
|
||||
use crate::render_observability::record_catalog_render;
|
||||
use crate::selection::collect_explicit_skill_mentions;
|
||||
use crate::shadow_selection_experiment::ShadowSelectionExperiment;
|
||||
use crate::sources::SkillProviders;
|
||||
@@ -76,6 +79,8 @@ struct RenderedCatalog {
|
||||
}
|
||||
|
||||
fn render_catalog(
|
||||
extension_metrics: Option<&dyn ExtensionMetrics>,
|
||||
catalog_surface: CatalogSurface,
|
||||
catalog: &SkillCatalog,
|
||||
include_skills_usage_instructions: bool,
|
||||
policy: SkillCatalogRenderPolicy,
|
||||
@@ -84,6 +89,7 @@ fn render_catalog(
|
||||
let Some(rendered) = render_available_skills(catalog, policy, budget) else {
|
||||
return RenderedCatalog::default();
|
||||
};
|
||||
record_catalog_render(extension_metrics, catalog_surface, budget, &rendered.report);
|
||||
let warning_message = rendered.report.warning_message();
|
||||
let fragment = rendered.into_fragment(include_skills_usage_instructions);
|
||||
RenderedCatalog {
|
||||
@@ -100,6 +106,7 @@ where
|
||||
Box::pin(async move {
|
||||
input.session_store.insert(SkillsSessionState {
|
||||
mcp_resources: input.mcp_resource_client.clone(),
|
||||
extension_metrics: input.extension_metrics.clone(),
|
||||
});
|
||||
let orchestrator_skills_available = !input
|
||||
.environments
|
||||
@@ -178,7 +185,12 @@ where
|
||||
let include_usage = thread_store
|
||||
.get::<ModelInfo>()
|
||||
.is_some_and(|model_info| model_info.include_skills_usage_instructions);
|
||||
let extension_metrics = session_store
|
||||
.get::<SkillsSessionState>()
|
||||
.and_then(|state| state.extension_metrics.clone());
|
||||
let rendered = render_catalog(
|
||||
extension_metrics.as_deref(),
|
||||
CatalogSurface::ThreadContext,
|
||||
&catalog,
|
||||
include_usage,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
@@ -234,8 +246,11 @@ where
|
||||
.as_deref()
|
||||
.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,
|
||||
@@ -266,6 +281,7 @@ where
|
||||
config.include_instructions,
|
||||
include_usage,
|
||||
metadata_budget,
|
||||
extension_metrics,
|
||||
));
|
||||
}
|
||||
sections
|
||||
@@ -346,6 +362,7 @@ where
|
||||
fn contribute<'a>(
|
||||
&'a self,
|
||||
input: TurnInputContext,
|
||||
extension_metrics: Option<Arc<dyn ExtensionMetrics>>,
|
||||
session_store: &'a ExtensionData,
|
||||
thread_store: &'a ExtensionData,
|
||||
turn_store: &'a ExtensionData,
|
||||
@@ -416,6 +433,8 @@ where
|
||||
.and_then(ModelInfo::resolved_context_window);
|
||||
let metadata_budget = capped_skill_metadata_budget(context_window);
|
||||
let rendered = render_catalog(
|
||||
extension_metrics.as_deref(),
|
||||
CatalogSurface::TurnInput,
|
||||
&turn_catalog,
|
||||
include_usage,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
|
||||
@@ -5,6 +5,7 @@ mod extension;
|
||||
mod fragments;
|
||||
pub mod provider;
|
||||
mod render;
|
||||
mod render_observability;
|
||||
mod selection;
|
||||
mod shadow_selection_experiment;
|
||||
mod sources;
|
||||
|
||||
@@ -121,7 +121,7 @@ impl SkillRenderReport {
|
||||
.then(|| SKILL_DESCRIPTION_TRUNCATED_WARNING.to_string())
|
||||
}
|
||||
|
||||
fn average_truncated_description_chars(&self) -> usize {
|
||||
pub(crate) fn average_truncated_description_chars(&self) -> usize {
|
||||
if self.total_count == 0 || self.truncated_description_chars == 0 {
|
||||
return 0;
|
||||
}
|
||||
@@ -158,7 +158,7 @@ fn metadata_line_cost(budget: SkillMetadataBudget, line: &str) -> usize {
|
||||
}
|
||||
|
||||
impl SkillMetadataBudget {
|
||||
fn limit(self) -> usize {
|
||||
pub(crate) fn limit(self) -> usize {
|
||||
match self {
|
||||
Self::Tokens(limit) | Self::Characters(limit) => limit,
|
||||
}
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
use codex_extension_api::ExtensionMetrics;
|
||||
use codex_otel::THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC;
|
||||
use codex_otel::THREAD_SKILLS_ENABLED_TOTAL_METRIC;
|
||||
use codex_otel::THREAD_SKILLS_KEPT_TOTAL_METRIC;
|
||||
use codex_otel::THREAD_SKILLS_TRUNCATED_METRIC;
|
||||
|
||||
use crate::render::SkillMetadataBudget;
|
||||
use crate::render::SkillRenderReport;
|
||||
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
pub(crate) enum CatalogSurface {
|
||||
ThreadContext,
|
||||
ExecutorWorldState,
|
||||
HostWorldState,
|
||||
TurnInput,
|
||||
}
|
||||
|
||||
impl CatalogSurface {
|
||||
fn as_str(self) -> &'static str {
|
||||
match self {
|
||||
Self::ThreadContext => "thread_context",
|
||||
Self::ExecutorWorldState => "executor_world_state",
|
||||
Self::HostWorldState => "host_world_state",
|
||||
Self::TurnInput => "turn_input",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn record_catalog_render(
|
||||
extension_metrics: Option<&dyn ExtensionMetrics>,
|
||||
catalog_surface: CatalogSurface,
|
||||
budget: SkillMetadataBudget,
|
||||
report: &SkillRenderReport,
|
||||
) {
|
||||
record_catalog_metrics(
|
||||
extension_metrics,
|
||||
catalog_surface,
|
||||
report.total_count,
|
||||
report.included_count,
|
||||
report.omitted_count,
|
||||
report.truncated_description_chars,
|
||||
);
|
||||
|
||||
if report.omitted_count > 0 || report.truncated_description_chars > 0 {
|
||||
tracing::info!(
|
||||
budget_limit = budget.limit(),
|
||||
total_skills = report.total_count,
|
||||
included_skills = report.included_count,
|
||||
omitted_skills = report.omitted_count,
|
||||
truncated_description_chars_per_skill = report.average_truncated_description_chars(),
|
||||
truncated_skill_descriptions = report.truncated_description_count,
|
||||
"truncated skill metadata to fit skills context budget"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn record_catalog_metrics(
|
||||
extension_metrics: Option<&dyn ExtensionMetrics>,
|
||||
catalog_surface: CatalogSurface,
|
||||
total_count: usize,
|
||||
included_count: usize,
|
||||
omitted_count: usize,
|
||||
truncated_description_chars: usize,
|
||||
) {
|
||||
let Some(extension_metrics) = extension_metrics else {
|
||||
return;
|
||||
};
|
||||
let samples = [
|
||||
(
|
||||
THREAD_SKILLS_ENABLED_TOTAL_METRIC,
|
||||
i64::try_from(total_count).unwrap_or(i64::MAX),
|
||||
),
|
||||
(
|
||||
THREAD_SKILLS_KEPT_TOTAL_METRIC,
|
||||
i64::try_from(included_count).unwrap_or(i64::MAX),
|
||||
),
|
||||
(
|
||||
THREAD_SKILLS_TRUNCATED_METRIC,
|
||||
if omitted_count > 0 { 1 } else { 0 },
|
||||
),
|
||||
(
|
||||
THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC,
|
||||
i64::try_from(truncated_description_chars).unwrap_or(i64::MAX),
|
||||
),
|
||||
];
|
||||
let tags = [("catalog_surface", catalog_surface.as_str())];
|
||||
for (name, value) in samples {
|
||||
extension_metrics.histogram(name, value, &tags);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "render_observability_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,82 @@
|
||||
use std::sync::Mutex;
|
||||
|
||||
use codex_extension_api::ExtensionMetrics;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::*;
|
||||
|
||||
#[derive(Debug, Eq, PartialEq)]
|
||||
struct RecordedHistogram {
|
||||
name: String,
|
||||
value: i64,
|
||||
tags: Vec<(String, String)>,
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
struct RecordingMetrics {
|
||||
samples: Mutex<Vec<RecordedHistogram>>,
|
||||
}
|
||||
|
||||
impl ExtensionMetrics for RecordingMetrics {
|
||||
fn histogram(&self, name: &str, value: i64, tags: &[(&str, &str)]) {
|
||||
self.samples
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.push(RecordedHistogram {
|
||||
name: name.to_string(),
|
||||
value,
|
||||
tags: tags
|
||||
.iter()
|
||||
.map(|(key, value)| ((*key).to_string(), (*value).to_string()))
|
||||
.collect(),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn records_core_equivalent_catalog_render_metrics_with_surface() {
|
||||
let metrics = RecordingMetrics::default();
|
||||
let report = SkillRenderReport {
|
||||
total_count: 5,
|
||||
included_count: 3,
|
||||
omitted_count: 2,
|
||||
truncated_description_chars: 700,
|
||||
truncated_description_count: 4,
|
||||
};
|
||||
|
||||
record_catalog_render(
|
||||
Some(&metrics),
|
||||
CatalogSurface::TurnInput,
|
||||
SkillMetadataBudget::Tokens(400),
|
||||
&report,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
*metrics
|
||||
.samples
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner),
|
||||
vec![
|
||||
RecordedHistogram {
|
||||
name: THREAD_SKILLS_ENABLED_TOTAL_METRIC.to_string(),
|
||||
value: 5,
|
||||
tags: vec![("catalog_surface".to_string(), "turn_input".to_string())],
|
||||
},
|
||||
RecordedHistogram {
|
||||
name: THREAD_SKILLS_KEPT_TOTAL_METRIC.to_string(),
|
||||
value: 3,
|
||||
tags: vec![("catalog_surface".to_string(), "turn_input".to_string())],
|
||||
},
|
||||
RecordedHistogram {
|
||||
name: THREAD_SKILLS_TRUNCATED_METRIC.to_string(),
|
||||
value: 1,
|
||||
tags: vec![("catalog_surface".to_string(), "turn_input".to_string())],
|
||||
},
|
||||
RecordedHistogram {
|
||||
name: THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC.to_string(),
|
||||
value: 700,
|
||||
tags: vec![("catalog_surface".to_string(), "turn_input".to_string())],
|
||||
},
|
||||
]
|
||||
);
|
||||
}
|
||||
@@ -4,6 +4,7 @@ use std::future::Future;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
|
||||
use codex_extension_api::ExtensionMetrics;
|
||||
use codex_mcp::McpResourceClient;
|
||||
use codex_mcp::McpResourceClientCacheKey;
|
||||
use codex_protocol::capabilities::SelectedCapabilityRoot;
|
||||
@@ -29,6 +30,7 @@ const MAX_CACHED_ORCHESTRATOR_CONTENT_BYTES: usize = 8 * 1024 * 1024;
|
||||
|
||||
pub(crate) struct SkillsSessionState {
|
||||
pub(crate) mcp_resources: Option<Arc<McpResourceClient>>,
|
||||
pub(crate) extension_metrics: Option<Arc<dyn ExtensionMetrics>>,
|
||||
}
|
||||
|
||||
pub(crate) struct SkillsThreadState {
|
||||
|
||||
@@ -1,7 +1,10 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_core_skills::HostSkillsSnapshot;
|
||||
use codex_core_skills::build_available_skills;
|
||||
use codex_core_skills::render::SkillRenderSideEffects;
|
||||
use codex_extension_api::ContextualUserFragment;
|
||||
use codex_extension_api::ExtensionMetrics;
|
||||
use codex_extension_api::PreviousWorldStateSection;
|
||||
use codex_extension_api::RenderedWorldStateFragment;
|
||||
use codex_extension_api::WorldStateSectionContribution;
|
||||
@@ -11,6 +14,8 @@ use serde_json::json;
|
||||
|
||||
use crate::fragments::AvailableSkillsInstructions;
|
||||
use crate::render::SkillMetadataBudget;
|
||||
use crate::render_observability::CatalogSurface;
|
||||
use crate::render_observability::record_catalog_metrics;
|
||||
|
||||
pub(crate) const SKILLS_WORLD_STATE_ID: &str = "skills";
|
||||
pub(crate) const HOST_SKILLS_WORLD_STATE_ID: &str = "host_skills";
|
||||
@@ -78,6 +83,7 @@ pub(crate) fn host_skills_world_state_section(
|
||||
include_instructions: bool,
|
||||
include_skills_usage_instructions: bool,
|
||||
metadata_budget: SkillMetadataBudget,
|
||||
extension_metrics: Option<Arc<dyn ExtensionMetrics>>,
|
||||
) -> WorldStateSectionContribution {
|
||||
let outcome = host_snapshot.outcome();
|
||||
let metadata_budget = match metadata_budget {
|
||||
@@ -91,6 +97,7 @@ pub(crate) fn host_skills_world_state_section(
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let render_report = available.as_ref().map(|available| available.report.clone());
|
||||
let body = available.map(|available| {
|
||||
AvailableSkillsInstructions::from_available_skills(
|
||||
available,
|
||||
@@ -121,6 +128,16 @@ pub(crate) fn host_skills_world_state_section(
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(report) = &render_report {
|
||||
record_catalog_metrics(
|
||||
extension_metrics.as_deref(),
|
||||
CatalogSurface::HostWorldState,
|
||||
report.total_count,
|
||||
report.included_count,
|
||||
report.omitted_count,
|
||||
report.truncated_description_chars,
|
||||
);
|
||||
}
|
||||
let body = match body.as_deref() {
|
||||
Some(body) => body,
|
||||
None if previous_is_absent => return None,
|
||||
|
||||
Reference in new issue
Block a user