chore: update exec crate to use std::time instead of chrono (#952)

When I originally wrote `elapsed.rs`, I realized we were using both
`std::time` and `chrono` with no real benefit of having both. We should
try to keep the `exec` subcommand trim (as it also buildable as a
standalone executable), so this helps tighten things up.
This commit is contained in:
Michael Bolin
2025-05-16 08:14:50 -07:00
committed by GitHub
parent 070499f534
commit 30cbfdfa87
4 changed files with 36 additions and 37 deletions

1
codex-rs/Cargo.lock generated
View File

@@ -503,7 +503,6 @@ dependencies = [
name = "codex-common" name = "codex-common"
version = "0.0.0" version = "0.0.0"
dependencies = [ dependencies = [
"chrono",
"clap", "clap",
"codex-core", "codex-core",
] ]

View File

@@ -7,11 +7,10 @@ edition = "2024"
workspace = true workspace = true
[dependencies] [dependencies]
chrono = { version = "0.4.40", optional = true }
clap = { version = "4", features = ["derive", "wrap_help"], optional = true } clap = { version = "4", features = ["derive", "wrap_help"], optional = true }
codex-core = { path = "../core" } codex-core = { path = "../core" }
[features] [features]
# Separate feature so that `clap` is not a mandatory dependency. # Separate feature so that `clap` is not a mandatory dependency.
cli = ["clap"] cli = ["clap"]
elapsed = ["chrono"] elapsed = []

View File

@@ -1,18 +1,19 @@
use chrono::Utc; use std::time::Duration;
use std::time::Instant;
/// Returns a string representing the elapsed time since `start_time` like /// Returns a string representing the elapsed time since `start_time` like
/// "1m15s" or "1.50s". /// "1m15s" or "1.50s".
pub fn format_elapsed(start_time: chrono::DateTime<Utc>) -> String { pub fn format_elapsed(start_time: Instant) -> String {
let elapsed = Utc::now().signed_duration_since(start_time); format_duration(start_time.elapsed())
format_time_delta(elapsed)
} }
fn format_time_delta(elapsed: chrono::TimeDelta) -> String { /// Convert a [`std::time::Duration`] into a human-readable, compact string.
let millis = elapsed.num_milliseconds(); ///
format_elapsed_millis(millis) /// Formatting rules:
} /// * < 1 s -> "{milli}ms"
/// * < 60 s -> "{sec:.2}s" (two decimal places)
pub fn format_duration(duration: std::time::Duration) -> String { /// * >= 60 s -> "{min}m{sec:02}s"
pub fn format_duration(duration: Duration) -> String {
let millis = duration.as_millis() as i64; let millis = duration.as_millis() as i64;
format_elapsed_millis(millis) format_elapsed_millis(millis)
} }
@@ -32,41 +33,40 @@ fn format_elapsed_millis(millis: i64) -> String {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use chrono::Duration;
#[test] #[test]
fn test_format_time_delta_subsecond() { fn test_format_duration_subsecond() {
// Durations < 1s should be rendered in milliseconds with no decimals. // Durations < 1s should be rendered in milliseconds with no decimals.
let dur = Duration::milliseconds(250); let dur = Duration::from_millis(250);
assert_eq!(format_time_delta(dur), "250ms"); assert_eq!(format_duration(dur), "250ms");
// Exactly zero should still work. // Exactly zero should still work.
let dur_zero = Duration::milliseconds(0); let dur_zero = Duration::from_millis(0);
assert_eq!(format_time_delta(dur_zero), "0ms"); assert_eq!(format_duration(dur_zero), "0ms");
} }
#[test] #[test]
fn test_format_time_delta_seconds() { fn test_format_duration_seconds() {
// Durations between 1s (inclusive) and 60s (exclusive) should be // Durations between 1s (inclusive) and 60s (exclusive) should be
// printed with 2-decimal-place seconds. // printed with 2-decimal-place seconds.
let dur = Duration::milliseconds(1_500); // 1.5s let dur = Duration::from_millis(1_500); // 1.5s
assert_eq!(format_time_delta(dur), "1.50s"); assert_eq!(format_duration(dur), "1.50s");
// 59.999s rounds to 60.00s // 59.999s rounds to 60.00s
let dur2 = Duration::milliseconds(59_999); let dur2 = Duration::from_millis(59_999);
assert_eq!(format_time_delta(dur2), "60.00s"); assert_eq!(format_duration(dur2), "60.00s");
} }
#[test] #[test]
fn test_format_time_delta_minutes() { fn test_format_duration_minutes() {
// Durations ≥ 1 minute should be printed mmss. // Durations ≥ 1 minute should be printed mmss.
let dur = Duration::milliseconds(75_000); // 1m15s let dur = Duration::from_millis(75_000); // 1m15s
assert_eq!(format_time_delta(dur), "1m15s"); assert_eq!(format_duration(dur), "1m15s");
let dur_exact = Duration::milliseconds(60_000); // 1m0s let dur_exact = Duration::from_millis(60_000); // 1m0s
assert_eq!(format_time_delta(dur_exact), "1m00s"); assert_eq!(format_duration(dur_exact), "1m00s");
let dur_long = Duration::milliseconds(3_601_000); let dur_long = Duration::from_millis(3_601_000);
assert_eq!(format_time_delta(dur_long), "60m01s"); assert_eq!(format_duration(dur_long), "60m01s");
} }
} }

View File

@@ -17,6 +17,7 @@ use owo_colors::OwoColorize;
use owo_colors::Style; use owo_colors::Style;
use shlex::try_join; use shlex::try_join;
use std::collections::HashMap; use std::collections::HashMap;
use std::time::Instant;
/// This should be configurable. When used in CI, users may not want to impose /// This should be configurable. When used in CI, users may not want to impose
/// a limit so they can see the full transcript. /// a limit so they can see the full transcript.
@@ -76,7 +77,7 @@ impl EventProcessor {
struct ExecCommandBegin { struct ExecCommandBegin {
command: Vec<String>, command: Vec<String>,
start_time: chrono::DateTime<Utc>, start_time: Instant,
} }
/// Metadata captured when an `McpToolCallBegin` event is received. /// Metadata captured when an `McpToolCallBegin` event is received.
@@ -84,11 +85,11 @@ struct McpToolCallBegin {
/// Formatted invocation string, e.g. `server.tool({"city":"sf"})`. /// Formatted invocation string, e.g. `server.tool({"city":"sf"})`.
invocation: String, invocation: String,
/// Timestamp when the call started so we can compute duration later. /// Timestamp when the call started so we can compute duration later.
start_time: chrono::DateTime<Utc>, start_time: Instant,
} }
struct PatchApplyBegin { struct PatchApplyBegin {
start_time: chrono::DateTime<Utc>, start_time: Instant,
auto_approved: bool, auto_approved: bool,
} }
@@ -133,7 +134,7 @@ impl EventProcessor {
call_id.clone(), call_id.clone(),
ExecCommandBegin { ExecCommandBegin {
command: command.clone(), command: command.clone(),
start_time: Utc::now(), start_time: Instant::now(),
}, },
); );
ts_println!( ts_println!(
@@ -208,7 +209,7 @@ impl EventProcessor {
call_id.clone(), call_id.clone(),
McpToolCallBegin { McpToolCallBegin {
invocation: invocation.clone(), invocation: invocation.clone(),
start_time: Utc::now(), start_time: Instant::now(),
}, },
); );
@@ -263,7 +264,7 @@ impl EventProcessor {
self.call_id_to_patch.insert( self.call_id_to_patch.insert(
call_id.clone(), call_id.clone(),
PatchApplyBegin { PatchApplyBegin {
start_time: Utc::now(), start_time: Instant::now(),
auto_approved, auto_approved,
}, },
); );