mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex-analytics] rework thread_source for thread analytics (#20949)
## Summary - make `thread_source` an explicit optional thread-level field on `thread/start`, `thread/fork`, and returned thread payloads - persist `thread_source` in rollout/session metadata so resumed live threads retain the original value - replace the old best-effort `session_source` -> `thread_source` mapping with an explicit caller-supplied analytics classification ## Why Before this change, analytics `thread_source` was populated by a best-effort mapping from `session_source`. `session_source` describes the runtime/client surface, not the actual thread-level origin, so that projection was not accurate enough to distinguish cases such as `user`, `subagent`, `memory_consolidation`, and future thread origins reliably. Making `thread_source` explicit keeps one thread-level analytics field while letting callers provide the real classification directly instead of recovering it indirectly from `session_source`. ## Impact For new analytics events, `thread_source` now reflects the explicit thread-level classification supplied by the caller rather than an inferred value derived from `session_source`. Existing protocol fields remain optional; callers that omit `threadSource` now produce `null` instead of a best-effort inferred value. ## Validation - `just write-app-server-schema` - `cargo test -p codex-analytics -p codex-core -p codex-app-server-protocol --no-run` - `cargo test -p codex-app-server-protocol generated_ts_optional_nullable_fields_only_in_params` - `cargo test -p codex-analytics thread_initialized_event_serializes_expected_shape` - `cargo test -p codex-core resume_stopped_thread_from_rollout_preserves_thread_source`
This commit is contained in:
@@ -2182,6 +2182,7 @@ mod tests {
|
||||
cwd: test_path_buf("/tmp").abs().into(),
|
||||
cli_version: "0.0.0".to_string(),
|
||||
source: SessionSource::Cli,
|
||||
thread_source: None,
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
agent_path: None,
|
||||
|
||||
@@ -307,6 +307,7 @@ impl ExternalAgentConfigRequestProcessor {
|
||||
config,
|
||||
initial_history: InitialHistory::Forked(rollout_items),
|
||||
session_source: None,
|
||||
thread_source: None,
|
||||
dynamic_tools: Vec::new(),
|
||||
persist_extended_history: false,
|
||||
metrics_service_name: None,
|
||||
|
||||
@@ -746,6 +746,7 @@ impl ThreadRequestProcessor {
|
||||
personality,
|
||||
ephemeral,
|
||||
session_start_source,
|
||||
thread_source,
|
||||
environments,
|
||||
persist_extended_history,
|
||||
} = params;
|
||||
@@ -799,6 +800,7 @@ impl ThreadRequestProcessor {
|
||||
typesafe_overrides,
|
||||
dynamic_tools,
|
||||
session_start_source,
|
||||
thread_source.map(Into::into),
|
||||
environment_selections,
|
||||
service_name,
|
||||
experimental_raw_events,
|
||||
@@ -882,6 +884,7 @@ impl ThreadRequestProcessor {
|
||||
typesafe_overrides: ConfigOverrides,
|
||||
dynamic_tools: Option<Vec<ApiDynamicToolSpec>>,
|
||||
session_start_source: Option<codex_app_server_protocol::ThreadStartSource>,
|
||||
thread_source: Option<codex_protocol::protocol::ThreadSource>,
|
||||
environments: Option<Vec<TurnEnvironmentSelection>>,
|
||||
service_name: Option<String>,
|
||||
experimental_raw_events: bool,
|
||||
@@ -998,6 +1001,7 @@ impl ThreadRequestProcessor {
|
||||
codex_app_server_protocol::ThreadStartSource::Clear => InitialHistory::Cleared,
|
||||
},
|
||||
session_source: None,
|
||||
thread_source,
|
||||
dynamic_tools: core_dynamic_tools,
|
||||
persist_extended_history: false,
|
||||
metrics_service_name: service_name,
|
||||
@@ -2382,6 +2386,11 @@ impl ThreadRequestProcessor {
|
||||
return Ok(());
|
||||
}
|
||||
};
|
||||
thread.thread_source = codex_thread
|
||||
.config_snapshot()
|
||||
.await
|
||||
.thread_source
|
||||
.map(Into::into);
|
||||
|
||||
self.thread_watch_manager
|
||||
.upsert_thread(thread.clone())
|
||||
@@ -2869,6 +2878,7 @@ impl ThreadRequestProcessor {
|
||||
base_instructions,
|
||||
developer_instructions,
|
||||
ephemeral,
|
||||
thread_source,
|
||||
exclude_turns,
|
||||
persist_extended_history,
|
||||
} = params;
|
||||
@@ -2959,6 +2969,7 @@ impl ThreadRequestProcessor {
|
||||
history: history_items.clone(),
|
||||
rollout_path: source_thread.rollout_path.clone(),
|
||||
}),
|
||||
thread_source.map(Into::into),
|
||||
/*persist_extended_history*/ false,
|
||||
self.request_trace_context(&request_id).await,
|
||||
)
|
||||
@@ -3018,6 +3029,11 @@ impl ThreadRequestProcessor {
|
||||
}
|
||||
thread
|
||||
};
|
||||
thread.thread_source = forked_thread
|
||||
.config_snapshot()
|
||||
.await
|
||||
.thread_source
|
||||
.map(Into::into);
|
||||
|
||||
self.thread_watch_manager
|
||||
.upsert_thread_silently(thread.clone())
|
||||
@@ -3620,6 +3636,7 @@ pub(crate) fn thread_from_stored_thread(
|
||||
agent_nickname: source.get_nickname(),
|
||||
agent_role: source.get_agent_role(),
|
||||
source: source.into(),
|
||||
thread_source: thread.thread_source.map(Into::into),
|
||||
git_info,
|
||||
name: thread.name,
|
||||
turns: Vec::new(),
|
||||
@@ -3682,6 +3699,7 @@ fn summary_from_state_db_metadata(
|
||||
cwd: PathBuf,
|
||||
cli_version: String,
|
||||
source: String,
|
||||
_thread_source: Option<codex_protocol::protocol::ThreadSource>,
|
||||
agent_nickname: Option<String>,
|
||||
agent_role: Option<String>,
|
||||
git_sha: Option<String>,
|
||||
@@ -3732,6 +3750,7 @@ fn summary_from_thread_metadata(metadata: &ThreadMetadata) -> ConversationSummar
|
||||
metadata.cwd.clone(),
|
||||
metadata.cli_version.clone(),
|
||||
metadata.source.clone(),
|
||||
metadata.thread_source,
|
||||
metadata.agent_nickname.clone(),
|
||||
metadata.agent_role.clone(),
|
||||
metadata.git_sha.clone(),
|
||||
@@ -3815,6 +3834,7 @@ fn build_thread_from_snapshot(
|
||||
agent_nickname: config_snapshot.session_source.get_nickname(),
|
||||
agent_role: config_snapshot.session_source.get_agent_role(),
|
||||
source: config_snapshot.session_source.clone().into(),
|
||||
thread_source: config_snapshot.thread_source.map(Into::into),
|
||||
git_info: None,
|
||||
name: None,
|
||||
turns: Vec::new(),
|
||||
|
||||
@@ -283,6 +283,7 @@ mod thread_processor_behavior_tests {
|
||||
cwd: PathBuf::from("/tmp"),
|
||||
cli_version: "0.0.0".to_string(),
|
||||
source: SessionSource::Cli,
|
||||
thread_source: Some(codex_protocol::protocol::ThreadSource::User),
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
agent_path: None,
|
||||
@@ -540,6 +541,7 @@ mod thread_processor_behavior_tests {
|
||||
reasoning_effort: None,
|
||||
personality: None,
|
||||
session_source: SessionSource::Cli,
|
||||
thread_source: None,
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
@@ -828,6 +830,7 @@ mod thread_processor_behavior_tests {
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
}),
|
||||
thread_source: Some(codex_protocol::protocol::ThreadSource::Subagent),
|
||||
agent_nickname: Some("atlas".to_string()),
|
||||
agent_role: Some("explorer".to_string()),
|
||||
model_provider: Some("test-provider".to_string()),
|
||||
@@ -849,6 +852,7 @@ mod thread_processor_behavior_tests {
|
||||
|
||||
assert_eq!(thread.agent_nickname, Some("atlas".to_string()));
|
||||
assert_eq!(thread.agent_role, Some("explorer".to_string()));
|
||||
assert_eq!(thread.thread_source, None);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -975,6 +979,7 @@ mod thread_processor_behavior_tests {
|
||||
PathBuf::from("/"),
|
||||
"0.0.0".to_string(),
|
||||
source,
|
||||
Some(codex_protocol::protocol::ThreadSource::Subagent),
|
||||
Some("atlas".to_string()),
|
||||
Some("explorer".to_string()),
|
||||
/*git_sha*/ None,
|
||||
|
||||
@@ -278,6 +278,7 @@ pub(crate) fn summary_to_thread(
|
||||
agent_nickname: source.get_nickname(),
|
||||
agent_role: source.get_agent_role(),
|
||||
source: source.into(),
|
||||
thread_source: None,
|
||||
git_info,
|
||||
name: None,
|
||||
turns: Vec::new(),
|
||||
|
||||
@@ -904,6 +904,7 @@ impl TurnRequestProcessor {
|
||||
history: parent_history.items,
|
||||
rollout_path: parent_thread.rollout_path(),
|
||||
}),
|
||||
/*thread_source*/ None,
|
||||
/*persist_extended_history*/ false,
|
||||
self.request_trace_context(request_id).await,
|
||||
)
|
||||
|
||||
@@ -902,6 +902,7 @@ mod tests {
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
source,
|
||||
thread_source: None,
|
||||
git_info: None,
|
||||
name: None,
|
||||
turns: Vec::new(),
|
||||
|
||||
Reference in New Issue
Block a user