diff --git a/docs/architecture/ard/ARCHITECTURE_RISKS_HEXAGONAL_PLAN.md b/docs/architecture/ard/ARCHITECTURE_RISKS_HEXAGONAL_PLAN.md index 58583b6..314faf3 100644 --- a/docs/architecture/ard/ARCHITECTURE_RISKS_HEXAGONAL_PLAN.md +++ b/docs/architecture/ard/ARCHITECTURE_RISKS_HEXAGONAL_PLAN.md @@ -328,6 +328,10 @@ Phase 5 artifacts (implemented): Exit criteria: - replay/compare core logic testable without terminal runtime. +Phase 6 artifacts (implemented): +- Terminal-independent replay/compare playback state transitions and range helpers: `src/application/replay_compare.rs` +- Replay/compare adapter loops narrowed to key handling + rendering while delegating state transitions: `src/app/replay/runner.rs`, `src/app/compare.rs`, `src/app/replay/state.rs` + ### Phase 7: Remove legacy coupling and enforce strict boundaries (1 week) 1. Deprecate direct `TesterArgs` use outside CLI adapter. 2. Remove now-obsolete conversion glue. diff --git a/src/app/compare.rs b/src/app/compare.rs index 176736f..c67a9d9 100644 --- a/src/app/compare.rs +++ b/src/app/compare.rs @@ -9,15 +9,16 @@ use std::time::Duration; use crossterm::event::{self, Event, KeyCode, KeyEventKind}; use tokio::sync::watch; +use crate::application::replay_compare::{ + PlaybackAction, PlaybackState, advance_playback, apply_playback_action, + clamp_window_to_records, records_range, resolve_step_ms, +}; use crate::args::CompareArgs; use crate::error::{AppError, AppResult, MetricsError}; -use crate::metrics::MetricRecord; use crate::ui::model::{CompareOverlay, UiData}; use crate::ui::render::setup_render_ui; -use super::replay::{ - ReplayWindow, SnapshotMarkers, build_ui_data_with_config, read_records_from_path, -}; +use super::replay::{SnapshotMarkers, build_ui_data_with_config, read_records_from_path}; use compare_output::print_compare_summary; /// Playback tick used when compare is in "playing" mode. @@ -41,8 +42,10 @@ pub(crate) async fn run_compare(args: &CompareArgs) -> AppResult<()> { left_records.sort_by_key(|record| record.elapsed_ms); right_records.sort_by_key(|record| record.elapsed_ms); - let (left_min, left_max) = records_range(&left_records); - let (right_min, right_max) = records_range(&right_records); + let (left_min, left_max) = records_range(&left_records) + .ok_or_else(|| AppError::metrics(MetricsError::ReplayRecordsEmpty))?; + let (right_min, right_max) = records_range(&right_records) + .ok_or_else(|| AppError::metrics(MetricsError::ReplayRecordsEmpty))?; let start_ms = left_min.min(right_min); let end_ms = left_max.max(right_max); @@ -84,18 +87,8 @@ pub(crate) async fn run_compare(args: &CompareArgs) -> AppResult<()> { let (ui_tx, _) = watch::channel(initial_ui); let render_ui_handle = setup_render_ui(&shutdown_tx, &ui_tx); - let mut state = ReplayWindow { - start_ms, - cursor_ms: start_ms, - end_ms, - playing: true, - }; - let step_ms = args - .replay_step - .unwrap_or(DEFAULT_COMPARE_STEP) - .as_millis() - .try_into() - .unwrap_or(1); + let mut state = PlaybackState::new(start_ms, end_ms); + let step_ms = resolve_step_ms(args.replay_step, DEFAULT_COMPARE_STEP); let mut last_tick = tokio::time::Instant::now(); let poll_interval = UI_POLL_INTERVAL; let mut dirty = true; @@ -117,45 +110,16 @@ pub(crate) async fn run_compare(args: &CompareArgs) -> AppResult<()> { { break; } - if matches!(key.code, KeyCode::Char(' ')) { - state.playing = !state.playing; - dirty = true; - } else if matches!(key.code, KeyCode::Left | KeyCode::Char('h')) { - state.playing = false; - state.cursor_ms = state.cursor_ms.saturating_sub(step_ms).max(state.start_ms); - dirty = true; - } else if matches!(key.code, KeyCode::Right | KeyCode::Char('l')) { - state.playing = false; - state.cursor_ms = state.cursor_ms.saturating_add(step_ms).min(state.end_ms); - dirty = true; - } else if matches!(key.code, KeyCode::Home) { - state.playing = false; - state.cursor_ms = state.start_ms; - dirty = true; - } else if matches!(key.code, KeyCode::End) { - state.playing = false; - state.cursor_ms = state.end_ms; - dirty = true; - } else if matches!(key.code, KeyCode::Char('r')) { - state.playing = false; - state.cursor_ms = state.start_ms; + if let Some(action) = resolve_playback_action(key.code) + && apply_playback_action(&mut state, action, step_ms) + { dirty = true; } } if state.playing { - let elapsed = last_tick.elapsed(); - if elapsed >= Duration::from_millis(COMPARE_TICK_MS) { - let tick_ms = u128::from(COMPARE_TICK_MS); - let steps = elapsed.as_millis().checked_div(tick_ms).unwrap_or(0); - let advance_ms = - u64::try_from(steps.saturating_mul(tick_ms)).unwrap_or(COMPARE_TICK_MS); - state.cursor_ms = state.cursor_ms.saturating_add(advance_ms).min(state.end_ms); + if advance_playback(&mut state, last_tick.elapsed(), COMPARE_TICK_MS) { last_tick = tokio::time::Instant::now(); - if state.cursor_ms >= state.end_ms { - state.cursor_ms = state.end_ms; - state.playing = false; - } dirty = true; } } else { @@ -163,8 +127,8 @@ pub(crate) async fn run_compare(args: &CompareArgs) -> AppResult<()> { } if dirty { - let left_state = clamped_window_state(&state, left_min, left_max); - let right_state = clamped_window_state(&state, right_min, right_max); + let left_state = clamp_window_to_records(&state, left_min, left_max); + let right_state = clamp_window_to_records(&state, right_min, right_max); let mut primary = build_ui_data_with_config( &left_records, args.expected_status_code, @@ -208,23 +172,6 @@ pub(crate) async fn run_compare(args: &CompareArgs) -> AppResult<()> { result } -fn records_range(records: &[MetricRecord]) -> (u64, u64) { - let min = records.first().map(|record| record.elapsed_ms).unwrap_or(0); - let max = records.last().map(|record| record.elapsed_ms).unwrap_or(0); - (min, max) -} - -fn clamped_window_state(base: &ReplayWindow, records_min: u64, records_max: u64) -> ReplayWindow { - let start_ms = base.start_ms.max(records_min).min(records_max); - let cursor_ms = base.cursor_ms.clamp(records_min, records_max); - ReplayWindow { - start_ms, - cursor_ms, - end_ms: records_max, - playing: base.playing, - } -} - fn resolve_label(path: &str, override_label: Option<&str>) -> String { if let Some(label) = override_label && !label.trim().is_empty() @@ -238,23 +185,24 @@ fn resolve_label(path: &str, override_label: Option<&str>) -> String { .unwrap_or_else(|| "compare".to_owned()) } -#[cfg(test)] -mod tests { - use super::clamped_window_state; - use crate::app::replay::ReplayWindow; - - #[test] - fn clamped_window_state_limits_cursor_to_dataset_range() { - let base = ReplayWindow { - start_ms: 0, - cursor_ms: 15_000, - end_ms: 20_000, - playing: true, - }; - let clamped = clamped_window_state(&base, 1_000, 10_000); - assert_eq!(clamped.start_ms, 1_000); - assert_eq!(clamped.cursor_ms, 10_000); - assert_eq!(clamped.end_ms, 10_000); - assert!(clamped.playing); +const fn resolve_playback_action(key_code: KeyCode) -> Option { + if matches!(key_code, KeyCode::Char(' ')) { + return Some(PlaybackAction::TogglePlayPause); + } + if matches!(key_code, KeyCode::Left | KeyCode::Char('h')) { + return Some(PlaybackAction::SeekBackward); + } + if matches!(key_code, KeyCode::Right | KeyCode::Char('l')) { + return Some(PlaybackAction::SeekForward); + } + if matches!(key_code, KeyCode::Home) { + return Some(PlaybackAction::SeekStart); + } + if matches!(key_code, KeyCode::End) { + return Some(PlaybackAction::SeekEnd); + } + if matches!(key_code, KeyCode::Char('r')) { + return Some(PlaybackAction::Restart); } + None } diff --git a/src/app/replay.rs b/src/app/replay.rs index dac6746..b4b839a 100644 --- a/src/app/replay.rs +++ b/src/app/replay.rs @@ -12,5 +12,5 @@ mod tests; pub(crate) use records::read_records_from_path; pub(crate) use runner::run_replay; use runner::window_slice; -pub(crate) use state::{ReplayWindow, SnapshotMarkers}; +pub(crate) use state::SnapshotMarkers; pub(crate) use ui::build_ui_data_with_config; diff --git a/src/app/replay/runner.rs b/src/app/replay/runner.rs index 6a3a604..da3553c 100644 --- a/src/app/replay/runner.rs +++ b/src/app/replay/runner.rs @@ -6,6 +6,9 @@ use std::time::Duration; use crossterm::event::{self, Event, KeyCode, KeyEventKind}; use tokio::sync::watch; +use crate::application::replay_compare::{ + PlaybackAction, PlaybackState, advance_playback, apply_playback_action, resolve_step_ms, +}; use crate::args::TesterArgs; use crate::error::{AppError, AppResult, MetricsError, ValidationError}; use crate::metrics::MetricRecord; @@ -17,7 +20,7 @@ use super::records::load_replay_records; use super::snapshots::{ SnapshotIntervalState, parse_snapshot_format, resolve_snapshot_range, resolve_snapshot_window, }; -use super::state::{ReplayWindow, SnapshotMarkers}; +use super::state::SnapshotMarkers; use super::ui::render_once; use super::{snapshots, ui}; @@ -70,12 +73,7 @@ pub(crate) async fn run_replay(args: &TesterArgs) -> AppResult<()> { || args.replay_snapshot_end.is_some() || args.replay_snapshot_out.is_some(); - let step_ms = args - .replay_step - .unwrap_or(DEFAULT_REPLAY_STEP) - .as_millis() - .try_into() - .unwrap_or(1); + let step_ms = resolve_step_ms(args.replay_step, DEFAULT_REPLAY_STEP); if !io::stdout().is_terminal() || args.no_ui { if snapshot_requested { @@ -126,12 +124,7 @@ pub(crate) async fn run_replay(args: &TesterArgs) -> AppResult<()> { let (ui_tx, _) = watch::channel(initial_ui); let render_ui_handle = setup_render_ui(&shutdown_tx, &ui_tx); - let mut state = ReplayWindow { - start_ms, - cursor_ms: start_ms, - end_ms, - playing: true, - }; + let mut state = PlaybackState::new(start_ms, end_ms); let mut last_tick = tokio::time::Instant::now(); let poll_interval = UI_POLL_INTERVAL; let mut dirty = true; @@ -157,29 +150,10 @@ pub(crate) async fn run_replay(args: &TesterArgs) -> AppResult<()> { { break; } - if matches!(key.code, KeyCode::Char(' ')) { - state.playing = !state.playing; - dirty = true; - } else if matches!(key.code, KeyCode::Left | KeyCode::Char('h')) { - state.playing = false; - state.cursor_ms = state.cursor_ms.saturating_sub(step_ms).max(state.start_ms); - dirty = true; - } else if matches!(key.code, KeyCode::Right | KeyCode::Char('l')) { - state.playing = false; - state.cursor_ms = state.cursor_ms.saturating_add(step_ms).min(state.end_ms); - dirty = true; - } else if matches!(key.code, KeyCode::Home) { - state.playing = false; - state.cursor_ms = state.start_ms; - dirty = true; - } else if matches!(key.code, KeyCode::End) { - state.playing = false; - state.cursor_ms = state.end_ms; - dirty = true; - } else if matches!(key.code, KeyCode::Char('r')) { - state.playing = false; - state.cursor_ms = state.start_ms; - dirty = true; + if let Some(action) = resolve_playback_action(key.code) { + if apply_playback_action(&mut state, action, step_ms) { + dirty = true; + } } else if matches!(key.code, KeyCode::Char('s')) { snapshot_markers.start = Some(state.cursor_ms); dirty = true; @@ -204,18 +178,8 @@ pub(crate) async fn run_replay(args: &TesterArgs) -> AppResult<()> { } if state.playing { - let elapsed = last_tick.elapsed(); - if elapsed >= Duration::from_millis(REPLAY_TICK_MS) { - let tick_ms = u128::from(REPLAY_TICK_MS); - let steps = elapsed.as_millis().checked_div(tick_ms).unwrap_or(0); - let advance_ms = - u64::try_from(steps.saturating_mul(tick_ms)).unwrap_or(REPLAY_TICK_MS); - state.cursor_ms = state.cursor_ms.saturating_add(advance_ms).min(state.end_ms); + if advance_playback(&mut state, last_tick.elapsed(), REPLAY_TICK_MS) { last_tick = tokio::time::Instant::now(); - if state.cursor_ms >= state.end_ms { - state.cursor_ms = state.end_ms; - state.playing = false; - } dirty = true; } } else { @@ -272,6 +236,28 @@ pub(crate) async fn run_replay(args: &TesterArgs) -> AppResult<()> { result } +const fn resolve_playback_action(key_code: KeyCode) -> Option { + if matches!(key_code, KeyCode::Char(' ')) { + return Some(PlaybackAction::TogglePlayPause); + } + if matches!(key_code, KeyCode::Left | KeyCode::Char('h')) { + return Some(PlaybackAction::SeekBackward); + } + if matches!(key_code, KeyCode::Right | KeyCode::Char('l')) { + return Some(PlaybackAction::SeekForward); + } + if matches!(key_code, KeyCode::Home) { + return Some(PlaybackAction::SeekStart); + } + if matches!(key_code, KeyCode::End) { + return Some(PlaybackAction::SeekEnd); + } + if matches!(key_code, KeyCode::Char('r')) { + return Some(PlaybackAction::Restart); + } + None +} + pub(super) fn window_slice( records: &[MetricRecord], start_ms: u64, diff --git a/src/app/replay/state.rs b/src/app/replay/state.rs index d143754..83651c6 100644 --- a/src/app/replay/state.rs +++ b/src/app/replay/state.rs @@ -1,10 +1,4 @@ -#[derive(Clone, Copy)] -pub(crate) struct ReplayWindow { - pub(crate) start_ms: u64, - pub(crate) cursor_ms: u64, - pub(crate) end_ms: u64, - pub(crate) playing: bool, -} +pub(crate) type ReplayWindow = crate::application::replay_compare::PlaybackState; #[derive(Default)] pub(crate) struct SnapshotMarkers { diff --git a/src/application/mod.rs b/src/application/mod.rs index 20f2795..2ce8401 100644 --- a/src/application/mod.rs +++ b/src/application/mod.rs @@ -1,3 +1,4 @@ pub(crate) mod commands; pub(crate) mod distributed_run; pub(crate) mod local_run; +pub(crate) mod replay_compare; diff --git a/src/application/replay_compare.rs b/src/application/replay_compare.rs new file mode 100644 index 0000000..7e489c5 --- /dev/null +++ b/src/application/replay_compare.rs @@ -0,0 +1,217 @@ +use std::time::Duration; + +use crate::metrics::MetricRecord; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct PlaybackState { + pub(crate) start_ms: u64, + pub(crate) cursor_ms: u64, + pub(crate) end_ms: u64, + pub(crate) playing: bool, +} + +impl PlaybackState { + #[must_use] + pub(crate) const fn new(start_ms: u64, end_ms: u64) -> Self { + Self { + start_ms, + cursor_ms: start_ms, + end_ms, + playing: true, + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum PlaybackAction { + TogglePlayPause, + SeekBackward, + SeekForward, + SeekStart, + SeekEnd, + Restart, +} + +#[must_use] +pub(crate) fn resolve_step_ms(step: Option, default_step: Duration) -> u64 { + let millis = step.unwrap_or(default_step).as_millis(); + u64::try_from(millis).unwrap_or(u64::MAX).max(1) +} + +pub(crate) fn apply_playback_action( + state: &mut PlaybackState, + action: PlaybackAction, + step_ms: u64, +) -> bool { + let previous = *state; + let step_ms = step_ms.max(1); + match action { + PlaybackAction::TogglePlayPause => { + state.playing = !state.playing; + } + PlaybackAction::SeekBackward => { + state.playing = false; + state.cursor_ms = state.cursor_ms.saturating_sub(step_ms).max(state.start_ms); + } + PlaybackAction::SeekForward => { + state.playing = false; + state.cursor_ms = state.cursor_ms.saturating_add(step_ms).min(state.end_ms); + } + PlaybackAction::SeekStart | PlaybackAction::Restart => { + state.playing = false; + state.cursor_ms = state.start_ms; + } + PlaybackAction::SeekEnd => { + state.playing = false; + state.cursor_ms = state.end_ms; + } + } + *state != previous +} + +pub(crate) fn advance_playback(state: &mut PlaybackState, elapsed: Duration, tick_ms: u64) -> bool { + if !state.playing || tick_ms == 0 { + return false; + } + if elapsed < Duration::from_millis(tick_ms) { + return false; + } + + let tick_ms = u128::from(tick_ms); + let steps = elapsed.as_millis().checked_div(tick_ms).unwrap_or(0); + if steps == 0 { + return false; + } + let advance_ms = u64::try_from(steps.saturating_mul(tick_ms)).unwrap_or(u64::MAX); + state.cursor_ms = state.cursor_ms.saturating_add(advance_ms).min(state.end_ms); + if state.cursor_ms >= state.end_ms { + state.cursor_ms = state.end_ms; + state.playing = false; + } + true +} + +#[must_use] +pub(crate) fn records_range(records: &[MetricRecord]) -> Option<(u64, u64)> { + let min = records.first().map(|record| record.elapsed_ms)?; + let max = records.last().map(|record| record.elapsed_ms)?; + Some((min, max)) +} + +#[must_use] +pub(crate) fn clamp_window_to_records( + base: &PlaybackState, + records_min: u64, + records_max: u64, +) -> PlaybackState { + let (min_ms, max_ms) = if records_min <= records_max { + (records_min, records_max) + } else { + (records_max, records_min) + }; + PlaybackState { + start_ms: base.start_ms.max(min_ms).min(max_ms), + cursor_ms: base.cursor_ms.clamp(min_ms, max_ms), + end_ms: max_ms, + playing: base.playing, + } +} + +#[cfg(test)] +mod tests { + use std::time::Duration; + + use crate::metrics::MetricRecord; + + use super::{ + PlaybackAction, PlaybackState, advance_playback, apply_playback_action, + clamp_window_to_records, records_range, resolve_step_ms, + }; + + fn metric_record(elapsed_ms: u64) -> MetricRecord { + MetricRecord { + elapsed_ms, + latency_ms: 1, + status_code: 200, + timed_out: false, + transport_error: false, + response_bytes: 0, + in_flight_ops: 0, + } + } + + #[test] + fn resolve_step_ms_defaults_and_clamps_zero() { + assert_eq!(resolve_step_ms(None, Duration::from_secs(1)), 1000); + assert_eq!( + resolve_step_ms(Some(Duration::from_millis(0)), Duration::from_secs(1)), + 1 + ); + } + + #[test] + fn apply_playback_action_handles_seek_and_restart() { + let mut state = PlaybackState { + start_ms: 1000, + cursor_ms: 5000, + end_ms: 9000, + playing: true, + }; + + let changed_back = apply_playback_action(&mut state, PlaybackAction::SeekBackward, 2500); + assert!(changed_back); + assert_eq!(state.cursor_ms, 2500); + assert!(!state.playing); + + let changed_forward = + apply_playback_action(&mut state, PlaybackAction::SeekForward, 10_000); + assert!(changed_forward); + assert_eq!(state.cursor_ms, 9000); + + let changed_restart = apply_playback_action(&mut state, PlaybackAction::Restart, 2500); + assert!(changed_restart); + assert_eq!(state.cursor_ms, 1000); + } + + #[test] + fn advance_playback_moves_cursor_and_stops_at_end() { + let mut state = PlaybackState { + start_ms: 0, + cursor_ms: 1000, + end_ms: 2500, + playing: true, + }; + + let changed_short_tick = advance_playback(&mut state, Duration::from_millis(500), 1000); + assert!(!changed_short_tick); + assert_eq!(state.cursor_ms, 1000); + assert!(state.playing); + + let changed_full_tick = advance_playback(&mut state, Duration::from_millis(2100), 1000); + assert!(changed_full_tick); + assert_eq!(state.cursor_ms, 2500); + assert!(!state.playing); + } + + #[test] + fn records_range_returns_min_and_max_when_present() { + let records = vec![metric_record(100), metric_record(500)]; + assert_eq!(records_range(&records), Some((100, 500))); + assert_eq!(records_range(&[]), None); + } + + #[test] + fn clamp_window_to_records_limits_state() { + let base = PlaybackState { + start_ms: 0, + cursor_ms: 15_000, + end_ms: 20_000, + playing: true, + }; + let clamped = clamp_window_to_records(&base, 1000, 10_000); + assert_eq!(clamped.start_ms, 1000); + assert_eq!(clamped.cursor_ms, 10_000); + assert_eq!(clamped.end_ms, 10_000); + assert!(clamped.playing); + } +}