From 9b43825f23e6670a61765ce80bda559b02908564 Mon Sep 17 00:00:00 2001 From: jif Date: Wed, 16 Sep 2026 15:07:28 +0000 Subject: [PATCH] Tag memory usage telemetry with the memory version (#45960) ## Why Memory usage telemetry identifies artifact kinds but does not distinguish reads from `memories` and `memories_v2`. ## What changed Return each artifact's memory version from shell command classification and add a `memory_version` tag (`v1` or `v2`) to usage counters. Normalize Windows path separators before classifying memory paths. ## Testing Extend regression coverage for both memory versions with Unix and PowerShell reads, and add a test for commands that read artifacts from both roots. GitOrigin-RevId: 68eaab3af0317613e4969e8c4e5478b18255ca52 --- codex-rs/core/src/memory_usage.rs | 12 +++++- codex-rs/memories/read/src/usage.rs | 44 ++++++++++++-------- codex-rs/memories/read/src/usage_tests.rs | 50 +++++++++++++++++------ 3 files changed, 74 insertions(+), 32 deletions(-) diff --git a/codex-rs/core/src/memory_usage.rs b/codex-rs/core/src/memory_usage.rs index 84afe8fb84..075a0dd87c 100644 --- a/codex-rs/core/src/memory_usage.rs +++ b/codex-rs/core/src/memory_usage.rs @@ -3,7 +3,8 @@ use crate::tools::context::ToolPayload; use crate::tools::flat_tool_name; use crate::tools::handlers::unified_exec::ExecCommandArgs; use codex_memories_read::usage::MEMORIES_USAGE_METRIC; -use codex_memories_read::usage::memories_usage_kinds_from_command; +use codex_memories_read::usage::memories_usage_from_command; +use codex_protocol::MemoryVersion; pub(crate) fn emit_metric_for_tool_read(invocation: &ToolInvocation, success: bool) { let Some(command) = shell_script_for_invocation(invocation) else { @@ -12,12 +13,19 @@ pub(crate) fn emit_metric_for_tool_read(invocation: &ToolInvocation, success: bo let success = if success { "true" } else { "false" }; let tool_name = flat_tool_name(&invocation.tool_name); - for kind in memories_usage_kinds_from_command(&command) { + for (kind, version) in memories_usage_from_command(&command) { invocation.turn.session_telemetry.counter( MEMORIES_USAGE_METRIC, /*inc*/ 1, &[ ("kind", kind.as_tag()), + ( + "memory_version", + match version { + MemoryVersion::V1 => "v1", + MemoryVersion::V2 => "v2", + }, + ), ("tool", tool_name.as_ref()), ("success", success), ], diff --git a/codex-rs/memories/read/src/usage.rs b/codex-rs/memories/read/src/usage.rs index 3cac16fb26..286ab1c8e5 100644 --- a/codex-rs/memories/read/src/usage.rs +++ b/codex-rs/memories/read/src/usage.rs @@ -1,3 +1,6 @@ +//! Best-effort classification of shell reads by memory artifact and root version. + +use codex_protocol::MemoryVersion; use codex_protocol::parse_command::ParsedCommand; use codex_shell_command::parse_command::parse_shell_script; @@ -24,7 +27,7 @@ impl MemoriesUsageKind { } } -pub fn memories_usage_kinds_from_command(command: &str) -> Vec { +pub fn memories_usage_from_command(command: &str) -> Vec<(MemoriesUsageKind, MemoryVersion)> { let commands = parse_shell_script(command); if commands .iter() @@ -36,28 +39,35 @@ pub fn memories_usage_kinds_from_command(command: &str) -> Vec get_memory_kind(path.display().to_string()), - ParsedCommand::Search { path, .. } => path.and_then(get_memory_kind), + ParsedCommand::Read { path, .. } => get_memory_usage(path.display().to_string()), + ParsedCommand::Search { path, .. } => path.and_then(get_memory_usage), ParsedCommand::ListFiles { .. } | ParsedCommand::Unknown { .. } => None, }) .collect() } -fn get_memory_kind(path: String) -> Option { - let path = path.replace("memories_v2/", "memories/"); - if path.contains("memories/MEMORY.md") { - Some(MemoriesUsageKind::MemoryMd) - } else if path.contains("memories/memory_summary.md") { - Some(MemoriesUsageKind::MemorySummary) - } else if path.contains("memories/raw_memories.md") { - Some(MemoriesUsageKind::RawMemories) - } else if path.contains("memories/rollout_summaries/") { - Some(MemoriesUsageKind::RolloutSummaries) - } else if path.contains("memories/skills/") { - Some(MemoriesUsageKind::Skills) +fn get_memory_usage(path: String) -> Option<(MemoriesUsageKind, MemoryVersion)> { + let path = path.replace('\\', "/"); + let version = if path.contains("memories_v2/") { + MemoryVersion::V2 } else { - None - } + MemoryVersion::V1 + }; + let path = path.replace("memories_v2/", "memories/"); + let kind = if path.contains("memories/MEMORY.md") { + MemoriesUsageKind::MemoryMd + } else if path.contains("memories/memory_summary.md") { + MemoriesUsageKind::MemorySummary + } else if path.contains("memories/raw_memories.md") { + MemoriesUsageKind::RawMemories + } else if path.contains("memories/rollout_summaries/") { + MemoriesUsageKind::RolloutSummaries + } else if path.contains("memories/skills/") { + MemoriesUsageKind::Skills + } else { + return None; + }; + Some((kind, version)) } #[cfg(test)] diff --git a/codex-rs/memories/read/src/usage_tests.rs b/codex-rs/memories/read/src/usage_tests.rs index 32e487ea8e..051945c55e 100644 --- a/codex-rs/memories/read/src/usage_tests.rs +++ b/codex-rs/memories/read/src/usage_tests.rs @@ -1,22 +1,46 @@ //! Regression coverage for versioned memory paths in shell usage telemetry. use super::MemoriesUsageKind; -use super::memories_usage_kinds_from_command; +use super::memories_usage_from_command; +use codex_protocol::MemoryVersion; use pretty_assertions::assert_eq; #[test] fn classifies_reads_in_both_memory_versions() { - for root in ["memories", "memories_v2"] { - let command = format!( - "cat /tmp/.codex/{root}/memory_summary.md && cat /tmp/.codex/{root}/rollout_summaries/thread.md" - ); - assert_eq!( - memories_usage_kinds_from_command(&command), - vec![ - MemoriesUsageKind::MemorySummary, - MemoriesUsageKind::RolloutSummaries, - ], - "memory root: {root}" - ); + for version in [MemoryVersion::V1, MemoryVersion::V2] { + let root = version.directory_name(); + for (reader, root) in [ + ("cat", format!("/tmp/.codex/{root}")), + ("Get-Content", format!(r"C:\Users\test\.codex\{root}")), + ] { + let command = format!( + "{reader} '{root}/memory_summary.md' && {reader} '{root}/rollout_summaries/thread.md'" + ); + assert_eq!( + memories_usage_from_command(&command), + vec![ + (MemoriesUsageKind::MemorySummary, version), + (MemoriesUsageKind::RolloutSummaries, version), + ], + "memory root: {root}" + ); + } } } + +#[test] +fn attributes_mixed_version_reads_to_their_roots() { + let command = "cat /tmp/.codex/memories/MEMORY.md \ + && sed -n '1,20p' /tmp/.codex/memories_v2/rollout_summaries/thread.md \ + && cat /tmp/.codex/memories/raw_memories.md \ + && cat /tmp/.codex/memories_v2/skills/testing/SKILL.md"; + assert_eq!( + memories_usage_from_command(command), + vec![ + (MemoriesUsageKind::MemoryMd, MemoryVersion::V1), + (MemoriesUsageKind::RolloutSummaries, MemoryVersion::V2), + (MemoriesUsageKind::RawMemories, MemoryVersion::V1), + (MemoriesUsageKind::Skills, MemoryVersion::V2), + ], + ); +}