From 34d1417e88f5ab232b28f30716aaab9dfa34e9bd Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 10 May 2026 20:10:04 +0000 Subject: [PATCH 1/2] refactor(cli): consolidate format helpers onto render::format / render::json (#329) `format_usd`, `format_int`, `format_uint`, `format_tokens`, and `coerce_whole_f64_to_int` were reimplemented in five places. Drop the `dead_code` allow on `render/format.rs` and `render/json.rs`, pull `format_tokens` into the canonical module, and route `overhead`, `compare`, `state`, `sessions`, `summary`, and `hotspots` through the shared helpers. Removes ~310 LOC net; golden snapshots are byte-identical. --- CHANGELOG.md | 4 + crates/relayburn-cli/src/commands/compare.rs | 63 +---- crates/relayburn-cli/src/commands/hotspots.rs | 5 +- crates/relayburn-cli/src/commands/overhead.rs | 262 ++---------------- crates/relayburn-cli/src/commands/sessions.rs | 5 +- crates/relayburn-cli/src/commands/state.rs | 55 ++-- crates/relayburn-cli/src/commands/summary.rs | 23 +- crates/relayburn-cli/src/render/format.rs | 55 ++-- crates/relayburn-cli/src/render/json.rs | 31 +-- 9 files changed, 97 insertions(+), 406 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d96226e..a5ac55ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ Cross-package release notes for relayburn. Package changelogs contain package-le ### Changed +- `relayburn-cli`: consolidated `format_usd` / `format_int` / `format_tokens` / + `coerce_whole_f64_to_int` duplicates across `overhead`, `compare`, `state`, + `sessions`, `summary`, and `hotspots` onto the canonical helpers in + `render::format` / `render::json`. No user-visible output changes. - `relayburn-sdk`: ledger query verbs (`query_turns`, `query_compactions`, `query_relationships`, `query_tool_result_events`, `query_user_turns`) now push `since` / `until` / `session_id` / `source` / `project` filters diff --git a/crates/relayburn-cli/src/commands/compare.rs b/crates/relayburn-cli/src/commands/compare.rs index 94edcd2f..d9e52fa2 100644 --- a/crates/relayburn-cli/src/commands/compare.rs +++ b/crates/relayburn-cli/src/commands/compare.rs @@ -21,6 +21,7 @@ use serde_json::{json, Value}; use crate::cli::{CompareArgs, GlobalArgs}; use crate::render::error::report_error; +use crate::render::format::{format_uint, format_usd}; use crate::render::json::render_json; use crate::render::progress::TaskProgress; @@ -492,44 +493,6 @@ fn days_to_ymd(days_from_epoch: i64) -> (i64, u32, u32) { // number formatting (matches packages/cli/src/format.ts) // --------------------------------------------------------------------------- -fn format_usd(n: f64) -> String { - if n == 0.0 { - return "$0.00".to_string(); - } - if n < 0.01 { - return format!("${}", to_fixed_raw(n, 4)); - } - if n < 1.0 { - return format!("${}", to_fixed_raw(n, 3)); - } - format!("${}", to_fixed_raw(n, 2)) -} - -/// Mirror JS `n.toFixed(d)` — keeps trailing zeros (so 1.5 with digits=2 -/// becomes "1.50"). Use this for human-readable output where the -/// fixed-width column matters; use [`to_fixed`] for JSON-bound values -/// that go through `Number(...).toString()` semantics. -fn to_fixed_raw(n: f64, digits: usize) -> String { - format!("{n:.*}", digits) -} - -fn format_int(n: u64) -> String { - // `toLocaleString('en-US')` thousands grouping. JS uses `,`. The - // golden corpus values are small (≤ 7) so the comma path isn't hit - // by the snapshot, but we implement it anyway for parity. - let s = n.to_string(); - let bytes = s.as_bytes(); - let mut out = String::with_capacity(s.len() + s.len() / 3); - let len = bytes.len(); - for (i, b) in bytes.iter().enumerate() { - if i > 0 && (len - i).is_multiple_of(3) { - out.push(','); - } - out.push(*b as char); - } - out -} - fn format_pct(rate: f64) -> String { // `Math.round(p * 100)` — round half to even on f64; matches JS for // the corpus we compare against (the `Math.round` half-to-even @@ -885,7 +848,7 @@ fn cell_fields(c: &CompareCell) -> [String; 3] { if c.no_data { return [DASH.to_string(), DASH.to_string(), DASH.to_string()]; } - let turns = format_int(c.turns); + let turns = format_uint(c.turns); let cost = c .cost_per_turn .map(format_usd) @@ -905,7 +868,7 @@ fn render_tty( ) -> String { let mut lines: Vec = Vec::new(); lines.push(String::new()); - lines.push(format!("turns analyzed: {}", format_int(analyzed_turns as u64))); + lines.push(format!("turns analyzed: {}", format_uint(analyzed_turns as u64))); let excluded = compute_excluded(summary, minimum); if excluded.total > 0 { @@ -1044,7 +1007,7 @@ fn render_tty( lines.push(format!( "{}: {} turns, {} total", display_model_name(m), - format_int(tot.turns), + format_uint(tot.turns), total_cost )); } @@ -1121,7 +1084,7 @@ fn format_excluded_note(excluded: &ExcludedBreakdown, minimum: FidelityClass) -> let noun = if excluded.total == 1 { "turn" } else { "turns" }; format!( "excluded {} {noun} below {} fidelity{breakdown}", - format_int(excluded.total), + format_uint(excluded.total), minimum.wire_str() ) } @@ -1130,22 +1093,6 @@ fn format_excluded_note(excluded: &ExcludedBreakdown, minimum: FidelityClass) -> mod tests { use super::*; - #[test] - fn format_usd_buckets() { - assert_eq!(format_usd(0.0), "$0.00"); - assert_eq!(format_usd(0.0017), "$0.0017"); - assert_eq!(format_usd(0.011), "$0.011"); - assert_eq!(format_usd(0.034), "$0.034"); - assert_eq!(format_usd(1.5), "$1.50"); - } - - #[test] - fn format_int_groups_thousands() { - assert_eq!(format_int(7), "7"); - assert_eq!(format_int(1_500), "1,500"); - assert_eq!(format_int(1_000_000), "1,000,000"); - } - #[test] fn format_pct_rounds_to_int() { assert_eq!(format_pct(0.0), "0%"); diff --git a/crates/relayburn-cli/src/commands/hotspots.rs b/crates/relayburn-cli/src/commands/hotspots.rs index a98aa344..e0c28e2b 100644 --- a/crates/relayburn-cli/src/commands/hotspots.rs +++ b/crates/relayburn-cli/src/commands/hotspots.rs @@ -36,6 +36,7 @@ use serde_json::{json, Map, Value}; use crate::cli::GlobalArgs; use crate::render::error::report_error; use crate::render::format::{coerce_whole_f64_to_int, format_uint, format_usd, render_table}; +use crate::render::json::render_json; use crate::render::progress::TaskProgress; const DEFAULT_TOP_N: usize = 10; @@ -265,9 +266,7 @@ fn run_inner(globals: &GlobalArgs, args: HotspotsArgs) -> anyhow::Result { fn emit_json(result: &HotspotsResult) { let mut value = hotspots_result_to_json(result); coerce_whole_f64_to_int(&mut value); - let mut out = serde_json::to_string_pretty(&value).unwrap_or_default(); - out.push('\n'); - print!("{}", out); + let _ = render_json(&value); } fn hotspots_result_to_json(result: &HotspotsResult) -> Value { diff --git a/crates/relayburn-cli/src/commands/overhead.rs b/crates/relayburn-cli/src/commands/overhead.rs index b635ef18..998ee342 100644 --- a/crates/relayburn-cli/src/commands/overhead.rs +++ b/crates/relayburn-cli/src/commands/overhead.rs @@ -16,6 +16,10 @@ use relayburn_sdk::{ use crate::cli::{GlobalArgs, OverheadAction, OverheadArgs}; use crate::render::error::report_error; +use crate::render::format::{ + coerce_whole_f64_to_int, format_tokens, format_uint, format_usd, render_table, +}; +use crate::render::json::render_json; use crate::render::progress::TaskProgress; pub fn run(globals: &GlobalArgs, args: OverheadArgs) -> i32 { @@ -68,7 +72,12 @@ fn run_report( } if globals.json { - if let Err(err) = render_json_ts_compatible(&result) { + let mut value = match serde_json::to_value(&result) { + Ok(v) => v, + Err(err) => return report_error(&io::Error::other(err), globals), + }; + coerce_whole_f64_to_int(&mut value); + if let Err(err) = render_json(&value) { return report_error(&err, globals); } return 0; @@ -124,7 +133,12 @@ fn run_trim( } if globals.json { - if let Err(err) = render_json_ts_compatible(&result) { + let mut value = match serde_json::to_value(&result) { + Ok(v) => v, + Err(err) => return report_error(&io::Error::other(err), globals), + }; + coerce_whole_f64_to_int(&mut value); + if let Err(err) = render_json(&value) { return report_error(&err, globals); } return 0; @@ -136,69 +150,6 @@ fn run_trim( 0 } -/// Serialize `value` as pretty-printed JSON with TS-compatible numeric output: -/// integer-valued `f64`s print as bare integers (`0` not `0.0`), matching -/// `JSON.stringify` semantics so the golden snapshots stay byte-equivalent. -fn render_json_ts_compatible(value: &T) -> io::Result<()> { - let mut json = serde_json::to_value(value).map_err(io::Error::other)?; - coerce_integer_floats(&mut json); - let stdout = io::stdout(); - let mut handle = stdout.lock(); - serde_json::to_writer_pretty(&mut handle, &json).map_err(io::Error::other)?; - handle.write_all(b"\n")?; - handle.flush() -} - -/// Walk a `serde_json::Value` tree and convert any `Number(f64)` whose value -/// is finite and exactly representable as `i64`/`u64` into the integer form. -/// Mirrors JavaScript's `JSON.stringify` numeric output, which always prints -/// `0` for `0.0_f64`. -/// -/// `Number::as_f64()` is lossy for exact integers above `2^53`, so the -/// `is_f64()` guard short-circuits before we ever touch a Number that -/// already serializes as an integer. Without the guard a u64 like -/// `9_007_199_254_740_993` (2^53 + 1) would silently become -/// `9_007_199_254_740_992` after a `to_value` → coerce → `to_writer_pretty` -/// round-trip. -fn coerce_integer_floats(v: &mut serde_json::Value) { - match v { - serde_json::Value::Number(n) => { - // Only coerce JSON Numbers that round-tripped as floats. - // `is_f64()` returns false for exact-integer Numbers (i64/u64), - // which already render without a decimal point and need no work. - if !n.is_f64() { - return; - } - if let Some(f) = n.as_f64() { - if f.is_finite() && f.fract() == 0.0 { - if f >= 0.0 && f <= u64::MAX as f64 { - let u = f as u64; - if (u as f64) == f { - *n = serde_json::Number::from(u); - } - } else if f >= i64::MIN as f64 && f < 0.0 { - let i = f as i64; - if (i as f64) == f { - *n = serde_json::Number::from(i); - } - } - } - } - } - serde_json::Value::Array(arr) => { - for item in arr.iter_mut() { - coerce_integer_floats(item); - } - } - serde_json::Value::Object(obj) => { - for (_k, val) in obj.iter_mut() { - coerce_integer_floats(val); - } - } - _ => {} - } -} - fn resolve_project(project: Option<&Path>) -> PathBuf { match project { Some(p) => match std::fs::canonicalize(p) { @@ -272,7 +223,7 @@ fn push_file_block( let applies_to = describe_applies_to(&parsed.applies_to); lines.push(format!( "{display} — {} lines, ~{} tokens — applies to: {applies_to}", - format_int(parsed.total_lines), + format_uint(parsed.total_lines), format_tokens(parsed.tokens), )); if parsed.tokens == 0 { @@ -292,7 +243,7 @@ fn push_file_block( lines.push(format!( " Cost over {since_label}: {} across {} session{}", format_usd(attribution.total_cost), - format_int(attribution.session_count), + format_uint(attribution.session_count), if attribution.session_count == 1 { "" } else { @@ -347,50 +298,7 @@ fn render_section_table(rows: &[OverheadSectionCost]) -> String { format!("{:.1}%", r.token_share * 100.0), ]); } - table_rows(&data) -} - -/// Mirror of the TS `format.ts::table` helper: pad each cell to the -/// widest cell in its column with two spaces between columns, trim -/// trailing whitespace per line. -fn table_rows(rows: &[Vec]) -> String { - if rows.is_empty() { - return String::new(); - } - let cols = rows.iter().map(|r| r.len()).max().unwrap_or(0); - let mut widths = vec![0usize; cols]; - for row in rows { - for (i, cell) in row.iter().enumerate() { - if cell.chars().count() > widths[i] { - widths[i] = cell.chars().count(); - } - } - } - let mut out = String::new(); - for (ridx, row) in rows.iter().enumerate() { - let mut line = String::new(); - for (i, cell) in row.iter().enumerate() { - if i > 0 { - line.push_str(" "); - } - line.push_str(cell); - // Pad to width if not last cell (so trailing whitespace can be - // trimmed away from the rightmost cell). - let pad = widths[i].saturating_sub(cell.chars().count()); - if i < row.len() - 1 { - for _ in 0..pad { - line.push(' '); - } - } - } - // Match TS `.trimEnd()` — strip trailing whitespace per row. - let trimmed = line.trim_end(); - out.push_str(trimmed); - if ridx + 1 < rows.len() { - out.push('\n'); - } - } - out + render_table(&data) } fn render_human_trim(result: &OverheadTrimResult) -> io::Result<()> { @@ -447,45 +355,6 @@ fn render_human_trim(result: &OverheadTrimResult) -> io::Result<()> { Ok(()) } -// --------------------------------------------------------------------------- -// Number / line-range formatters — mirror packages/cli/src/format.ts. -// --------------------------------------------------------------------------- - -fn format_usd(n: f64) -> String { - if n == 0.0 { - return "$0.00".to_string(); - } - if n < 0.01 { - return format!("${n:.4}"); - } - if n < 1.0 { - return format!("${n:.3}"); - } - format!("${n:.2}") -} - -fn format_int(n: u64) -> String { - // en-US locale grouping: every three digits separated by `,`. - let s = n.to_string(); - let bytes = s.as_bytes(); - let mut out = String::with_capacity(s.len() + s.len() / 3); - for (i, b) in bytes.iter().enumerate() { - if i > 0 && (bytes.len() - i).is_multiple_of(3) { - out.push(','); - } - out.push(*b as char); - } - out -} - -fn format_tokens(tokens: u64) -> String { - if tokens >= 1000 { - let v = tokens as f64 / 1000.0; - return format!("{v:.1}k"); - } - tokens.to_string() -} - fn format_line_range(start: u64, end: u64) -> String { let s = format!("{start:>4}"); let e = format!("{end:>4}"); @@ -496,102 +365,9 @@ fn format_line_range(start: u64, end: u64) -> String { mod tests { use super::*; - #[test] - fn format_usd_buckets_match_ts() { - assert_eq!(format_usd(0.0), "$0.00"); - assert_eq!(format_usd(0.001), "$0.0010"); - assert_eq!(format_usd(0.5), "$0.500"); - assert_eq!(format_usd(12.345), "$12.35"); - } - - #[test] - fn format_int_groups_thousands() { - assert_eq!(format_int(0), "0"); - assert_eq!(format_int(999), "999"); - assert_eq!(format_int(1_000), "1,000"); - assert_eq!(format_int(1_234_567), "1,234,567"); - } - - #[test] - fn format_tokens_collapses_kilos() { - assert_eq!(format_tokens(999), "999"); - assert_eq!(format_tokens(1_000), "1.0k"); - assert_eq!(format_tokens(2_500), "2.5k"); - } - #[test] fn format_line_range_pads_to_four() { assert_eq!(format_line_range(7, 11), " 7- 11"); assert_eq!(format_line_range(100, 200), " 100- 200"); } - - #[test] - fn table_rows_pads_columns() { - let rendered = table_rows(&[ - vec!["a".into(), "bb".into()], - vec!["ccc".into(), "d".into()], - ]); - assert_eq!(rendered, "a bb\nccc d"); - } - - // -- coerce_integer_floats ------------------------------------------------ - // - // The helper walks a `serde_json::Value` tree and converts whole-valued - // floats (e.g. `0.0`) into their integer forms so the pretty-printed JSON - // matches `JSON.stringify`'s "0" not "0.0" rendering. The cases below - // pin down its core invariants — the most important one being that it - // never lossily coerces an exact-integer JSON Number that happens to be - // larger than `2^53`. - - #[test] - fn coerce_integer_floats_leaves_exact_integers_untouched() { - // `Number::from(u64)` produces an exact-integer Number for which - // `is_f64()` returns false. Mutation must short-circuit so a - // round-trip through `to_value` is a no-op for integer fields. - let mut v = serde_json::json!({ "n": 42_u64 }); - coerce_integer_floats(&mut v); - assert_eq!(v, serde_json::json!({ "n": 42_u64 })); - } - - #[test] - fn coerce_integer_floats_preserves_large_u64_above_2_pow_53() { - // `2^53 + 1 = 9_007_199_254_740_993` — exactly representable as u64 - // but NOT as f64. Without the `is_f64()` guard the value would round - // down to 2^53 = 9_007_199_254_740_992 after a lossy `as_f64()` cast. - let huge: u64 = (1u64 << 53) + 1; - let mut v = serde_json::json!({ "n": huge }); - coerce_integer_floats(&mut v); - // Round-trip through to_string to assert the byte representation - // wasn't truncated by an unguarded f64 coercion. - let s = serde_json::to_string(&v).expect("serialize"); - assert!( - s.contains(&huge.to_string()), - "expected `{huge}` in `{s}`; large u64 was lossily coerced", - ); - } - - #[test] - fn coerce_integer_floats_collapses_whole_valued_floats() { - // `42.0_f64` parsed through `serde_json::Number::from_f64` is the - // case the helper exists to handle. After coercion the Number must - // serialize as the integer form, matching `JSON.stringify`. - let n = serde_json::Number::from_f64(42.0).expect("finite"); - let mut v = serde_json::Value::Number(n); - coerce_integer_floats(&mut v); - assert_eq!(serde_json::to_string(&v).expect("serialize"), "42"); - } - - #[test] - fn coerce_integer_floats_recurses_into_arrays_and_objects() { - // Smoke-cover the recursion arms — a nested whole-valued float - // inside an array inside an object should still flatten to its - // integer form. - let inner = serde_json::Number::from_f64(7.0).expect("finite"); - let mut v = serde_json::json!({ "outer": [serde_json::Value::Number(inner)] }); - coerce_integer_floats(&mut v); - assert_eq!( - serde_json::to_string(&v).expect("serialize"), - r#"{"outer":[7]}"# - ); - } } diff --git a/crates/relayburn-cli/src/commands/sessions.rs b/crates/relayburn-cli/src/commands/sessions.rs index 815eb71b..2215bc70 100644 --- a/crates/relayburn-cli/src/commands/sessions.rs +++ b/crates/relayburn-cli/src/commands/sessions.rs @@ -26,6 +26,7 @@ use serde_json::{json, Value}; use crate::cli::{GlobalArgs, SessionsArgs, SessionsListArgs, SessionsSubcommand}; use crate::render::error::report_error; use crate::render::format::{coerce_whole_f64_to_int, format_uint, format_usd, render_table}; +use crate::render::json::render_json; use crate::render::progress::TaskProgress; const DEFAULT_SINCE: &str = "7d"; @@ -109,9 +110,7 @@ fn emit_json(result: &SessionsListResult, since: &str, project: Option<&str>, gr "sessions": result.sessions, }); coerce_whole_f64_to_int(&mut payload); - let mut out = serde_json::to_string_pretty(&payload).unwrap_or_default(); - out.push('\n'); - print!("{}", out); + let _ = render_json(&payload); } fn emit_human(result: &SessionsListResult, since: &str, grep: Option<&str>) { diff --git a/crates/relayburn-cli/src/commands/state.rs b/crates/relayburn-cli/src/commands/state.rs index c6e477c0..be6f1998 100644 --- a/crates/relayburn-cli/src/commands/state.rs +++ b/crates/relayburn-cli/src/commands/state.rs @@ -15,6 +15,7 @@ use crate::cli::{ GlobalArgs, StateArgs, StateRebuildArgs, StateRebuildTarget, StateSubcommand, }; use crate::render::error::{report_error, report_ledger_error}; +use crate::render::format::format_uint; use crate::render::json::render_json; use crate::render::progress::TaskProgress; @@ -82,35 +83,35 @@ fn format_status(s: &StateStatus) -> String { } out.push_str(&format!( " tracked rows: {}\n", - format_int(s.burn.tracked_rows) + format_uint(s.burn.tracked_rows) )); out.push_str(&format!( " turns: {}\n", - format_int(s.burn.rows.turns) + format_uint(s.burn.rows.turns) )); out.push_str(&format!( " user_turns: {}\n", - format_int(s.burn.rows.user_turns) + format_uint(s.burn.rows.user_turns) )); out.push_str(&format!( " compactions: {}\n", - format_int(s.burn.rows.compactions) + format_uint(s.burn.rows.compactions) )); out.push_str(&format!( " relationships: {}\n", - format_int(s.burn.rows.relationships) + format_uint(s.burn.rows.relationships) )); out.push_str(&format!( " tool_result_events: {}\n", - format_int(s.burn.rows.tool_result_events) + format_uint(s.burn.rows.tool_result_events) )); out.push_str(&format!( " sessions: {}\n", - format_int(s.burn.rows.sessions) + format_uint(s.burn.rows.sessions) )); out.push_str(&format!( " stamps: {}\n", - format_int(s.burn.rows.stamps) + format_uint(s.burn.rows.stamps) )); out.push_str("content DB (content.sqlite):\n"); out.push_str(&format!( @@ -120,7 +121,7 @@ fn format_status(s: &StateStatus) -> String { if !s.content.exists { out.push_str(" status: not built yet\n"); } - out.push_str(&format!(" rows: {}\n", format_int(s.content.rows))); + out.push_str(&format!(" rows: {}\n", format_uint(s.content.rows))); out.push_str("archive state:\n"); out.push_str(&format!(" schema version: {}\n", s.archive.schema_version)); out.push_str(&format!( @@ -169,20 +170,6 @@ fn rel_to_home(path: &str, home: &str) -> String { format!("${{RELAYBURN_HOME}}/{}", rest) } -fn format_int(n: u64) -> String { - // Insert thousands separators with commas; matches the TS - // `formatInt`'s `Intl.NumberFormat('en-US')` output. - let s = n.to_string(); - let mut out = String::with_capacity(s.len() + s.len() / 3); - for (i, ch) in s.chars().rev().enumerate() { - if i > 0 && i % 3 == 0 { - out.push(','); - } - out.push(ch); - } - out.chars().rev().collect() -} - fn format_bytes(n: u64) -> String { if n < 1024 { return format!("{} bytes", n); @@ -265,8 +252,8 @@ fn run_rebuild_derivable(globals: &GlobalArgs) -> i32 { } else { println!( "rebuilt derivable state: dropped {} event rows + {} content rows", - format_int(summary.rows_dropped as u64), - format_int(summary.content_rows_dropped as u64), + format_uint(summary.rows_dropped as u64), + format_uint(summary.content_rows_dropped as u64), ); println!( " re-ingest from upstream session files via 'burn ingest' to \ @@ -395,7 +382,7 @@ fn run_prune(globals: &GlobalArgs, args: crate::cli::StatePruneArgs) -> i32 { } else { println!( "pruned {} content row{} ({})", - format_int(stats.rows_deleted as u64), + format_uint(stats.rows_deleted as u64), if stats.rows_deleted == 1 { "" } else { "s" }, format_bytes(stats.bytes_freed.max(0) as u64) ); @@ -555,20 +542,20 @@ fn print_reset_report( if executed { println!( "reset derived state: dropped {} event row{} + {} stamp{} + {} content row{}", - format_int(summary.rows_dropped as u64), + format_uint(summary.rows_dropped as u64), if summary.rows_dropped == 1 { "" } else { "s" }, - format_int(summary.stamps_dropped as u64), + format_uint(summary.stamps_dropped as u64), if summary.stamps_dropped == 1 { "" } else { "s" }, - format_int(summary.content_rows_dropped as u64), + format_uint(summary.content_rows_dropped as u64), if summary.content_rows_dropped == 1 { "" } else { "s" }, ); match ingest_report { Some(report) => { println!( " re-ingested {} session{} (+{} turn{}).", - format_int(report.ingested_sessions as u64), + format_uint(report.ingested_sessions as u64), if report.ingested_sessions == 1 { "" } else { "s" }, - format_int(report.appended_turns as u64), + format_uint(report.appended_turns as u64), if report.appended_turns == 1 { "" } else { "s" }, ); } @@ -582,11 +569,11 @@ fn print_reset_report( } else { println!( "burn state reset (dry run): would drop {} event row{} + {} stamp{} + {} content row{}.", - format_int(summary.rows_dropped as u64), + format_uint(summary.rows_dropped as u64), if summary.rows_dropped == 1 { "" } else { "s" }, - format_int(summary.stamps_dropped as u64), + format_uint(summary.stamps_dropped as u64), if summary.stamps_dropped == 1 { "" } else { "s" }, - format_int(summary.content_rows_dropped as u64), + format_uint(summary.content_rows_dropped as u64), if summary.content_rows_dropped == 1 { "" } else { "s" }, ); println!(" re-run with --force to actually wipe (add --reingest to repopulate)."); diff --git a/crates/relayburn-cli/src/commands/summary.rs b/crates/relayburn-cli/src/commands/summary.rs index afd63551..a5ccae89 100644 --- a/crates/relayburn-cli/src/commands/summary.rs +++ b/crates/relayburn-cli/src/commands/summary.rs @@ -33,6 +33,7 @@ use serde_json::{json, Map, Value}; use crate::cli::GlobalArgs; use crate::render::error::report_error; use crate::render::format::{coerce_whole_f64_to_int, format_uint, format_usd, render_table}; +use crate::render::json::render_json; use crate::render::progress::TaskProgress; /// Per-command flags for `burn summary`. Mirrors the TS surface in @@ -425,7 +426,7 @@ fn ingest_prelude_text(ingest_report: &relayburn_sdk::IngestReport) -> String { fn emit_json(report: &SummaryGroupedReport, ingest_report: &relayburn_sdk::IngestReport) { let value = grouped_json_value(report, ingest_report); - print_json(&value); + let _ = render_json(&value); } fn grouped_json_value( @@ -504,12 +505,6 @@ fn grouped_json_value( value } -fn print_json(value: &Value) { - let mut out = serde_json::to_string_pretty(value).unwrap_or_default(); - out.push('\n'); - print!("{}", out); -} - fn cost_breakdown_to_json(c: &CostBreakdown) -> Value { json!({ "model": c.model.as_ref(), @@ -570,7 +565,7 @@ fn render_by_tool_report( ); } coerce_whole_f64_to_int(&mut payload); - print_json(&payload); + let _ = render_json(&payload); return Ok(0); } @@ -640,7 +635,7 @@ fn render_subagent_type_report( if globals.json { let mut value = serde_json::to_value(stats)?; coerce_whole_f64_to_int(&mut value); - print_json(&value); + let _ = render_json(&value); return Ok(0); } @@ -704,7 +699,7 @@ fn render_relationship_report( if globals.json { let mut value = json!({ "relationships": report.relationships }); coerce_whole_f64_to_int(&mut value); - print_json(&value); + let _ = render_json(&value); return Ok(0); } @@ -753,7 +748,7 @@ fn render_relationship_subagent_report( "message": NO_RELATIONSHIPS_MESSAGE, }); coerce_whole_f64_to_int(&mut value); - print_json(&value); + let _ = render_json(&value); return Ok(0); } return Ok(render_no_relationships(globals)); @@ -764,7 +759,7 @@ fn render_relationship_subagent_report( "subagentTypes": report.subagent_types, }); coerce_whole_f64_to_int(&mut value); - print_json(&value); + let _ = render_json(&value); return Ok(0); } @@ -803,7 +798,7 @@ fn render_relationship_subagent_report( fn render_no_relationships(globals: &GlobalArgs) -> i32 { if globals.json { - print_json(&json!({ + let _ = render_json(&json!({ "relationships": [], "message": NO_RELATIONSHIPS_MESSAGE, })); @@ -827,7 +822,7 @@ fn render_subagent_tree_report( "root": root, }); coerce_whole_f64_to_int(&mut value); - print_json(&value); + let _ = render_json(&value); return Ok(0); } diff --git a/crates/relayburn-cli/src/render/format.rs b/crates/relayburn-cli/src/render/format.rs index 72f9043d..cd47aa63 100644 --- a/crates/relayburn-cli/src/render/format.rs +++ b/crates/relayburn-cli/src/render/format.rs @@ -6,13 +6,12 @@ //! module mirrors them in Rust. //! //! - [`format_usd`] — money rendering with three rate-tier precisions. -//! - [`format_int`] — `toLocaleString('en-US')` thousands-separator output. +//! - [`format_uint`] — `toLocaleString('en-US')` thousands-separator output. +//! - [`format_tokens`] — collapse 1k+ values to `.k`. //! - [`render_table`] — pad-end columns separated by `" "` (two spaces), //! `trim_end` each row, `\n`-joined. NOT the comfy-table preset; this //! is the spartan layout the TS CLI snapshots assume. -#![allow(dead_code)] - /// `formatUsd` from `packages/cli/src/format.ts`. Three rate tiers: /// - `0` → `$0.00` /// - `< 0.01` → `$` @@ -31,22 +30,11 @@ pub fn format_usd(n: f64) -> String { format!("${:.2}", n) } -/// `formatInt` from `packages/cli/src/format.ts`. JS `Number.toLocaleString('en-US')` -/// uses thousands separators (`,`) for non-negative integers and a leading `-` -/// for negatives. We mirror that without pulling in a locale dep. -pub fn format_int(n: i64) -> String { - if n < 0 { - return format!("-{}", format_int_unsigned(n.unsigned_abs())); - } - format_int_unsigned(n as u64) -} - -/// Convenience wrapper for unsigned token / count fields. +/// `formatInt` from `packages/cli/src/format.ts`. JS +/// `Number.toLocaleString('en-US')` uses thousands separators (`,`) +/// for non-negative integers. Token counts and row counts are all +/// unsigned, so we expose just the `u64` form. pub fn format_uint(n: u64) -> String { - format_int_unsigned(n) -} - -fn format_int_unsigned(n: u64) -> String { let raw = n.to_string(); let bytes = raw.as_bytes(); let len = bytes.len(); @@ -72,6 +60,17 @@ fn format_int_unsigned(n: u64) -> String { out } +/// Collapse token counts to a compact `.k` form once they reach +/// 1,000; sub-1k values render as plain integers. Mirrors the TS +/// `formatTokens` helper used by `burn overhead` output. +pub fn format_tokens(tokens: u64) -> String { + if tokens >= 1000 { + let v = tokens as f64 / 1000.0; + return format!("{v:.1}k"); + } + tokens.to_string() +} + /// `table()` from `packages/cli/src/format.ts`. Each column gets padded to /// the max width seen in that column; columns are joined with two spaces; /// trailing whitespace is trimmed per row. The first row is the header. @@ -197,18 +196,20 @@ mod tests { } #[test] - fn format_int_thousands() { - assert_eq!(format_int(0), "0"); - assert_eq!(format_int(999), "999"); - assert_eq!(format_int(1_000), "1,000"); - assert_eq!(format_int(5_100), "5,100"); - assert_eq!(format_int(19_500), "19,500"); - assert_eq!(format_int(1_000_000), "1,000,000"); + fn format_uint_thousands() { + assert_eq!(format_uint(0), "0"); + assert_eq!(format_uint(999), "999"); + assert_eq!(format_uint(1_000), "1,000"); + assert_eq!(format_uint(5_100), "5,100"); + assert_eq!(format_uint(19_500), "19,500"); + assert_eq!(format_uint(1_000_000), "1,000,000"); } #[test] - fn format_int_negative() { - assert_eq!(format_int(-1_500), "-1,500"); + fn format_tokens_collapses_kilos() { + assert_eq!(format_tokens(999), "999"); + assert_eq!(format_tokens(1_000), "1.0k"); + assert_eq!(format_tokens(2_500), "2.5k"); } #[test] diff --git a/crates/relayburn-cli/src/render/json.rs b/crates/relayburn-cli/src/render/json.rs index d490d275..bd654614 100644 --- a/crates/relayburn-cli/src/render/json.rs +++ b/crates/relayburn-cli/src/render/json.rs @@ -3,14 +3,12 @@ //! Writes a serializable value to stdout as JSON, with a trailing //! newline so shell pipelines see a clean line boundary. The TS CLI's //! `--json` mode is exactly this: a single JSON document per -//! invocation, no leading garbage. Wave 2 commands gate their human -//! renderer on `globals.json == false` and call [`render_json`] when -//! it's `true`. +//! invocation, no leading garbage. Commands gate their human renderer +//! on `globals.json == false` and call [`render_json`] when it's `true`. //! -//! Helpers here are unused on the scaffold branch; Wave 2 PRs are what -//! consume them. `#[allow(dead_code)]` keeps the surface intact. - -#![allow(dead_code)] +//! Callers that need `JSON.stringify` numeric semantics (where +//! whole-valued `f64`s print as bare integers) should run their value +//! through [`crate::render::format::coerce_whole_f64_to_int`] first. use std::io::{self, Write}; @@ -22,21 +20,7 @@ use serde::Serialize; pub fn render_json(value: &T) -> io::Result<()> { let stdout = io::stdout(); let mut handle = stdout.lock(); - serde_json::to_writer_pretty(&mut handle, value) - .map_err(io::Error::other)?; - handle.write_all(b"\n")?; - handle.flush() -} - -/// Compact-form variant. The TS CLI defaults to pretty-printed output -/// because `burn ... --json | jq` is the dominant invocation; compact -/// mode is here for embedded callers that want to pipe output through -/// `wc` or measure size budgets. -pub fn render_json_compact(value: &T) -> io::Result<()> { - let stdout = io::stdout(); - let mut handle = stdout.lock(); - serde_json::to_writer(&mut handle, value) - .map_err(io::Error::other)?; + serde_json::to_writer_pretty(&mut handle, value).map_err(io::Error::other)?; handle.write_all(b"\n")?; handle.flush() } @@ -46,12 +30,11 @@ mod tests { use super::*; use serde_json::json; - // Smoke test: the helpers should accept anything `Serialize` and + // Smoke test: the helper should accept anything `Serialize` and // not panic. Real I/O assertions live in the integration smoke // test under `tests/smoke.rs` which drives the binary end-to-end. #[test] fn render_json_accepts_arbitrary_serialize_input() { assert!(render_json(&json!({ "ok": true, "rows": [1, 2, 3] })).is_ok()); - assert!(render_json_compact(&json!({ "ok": true })).is_ok()); } } From 0e36ea721b7258af66325b16613a0dc8be5adcc1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 10 May 2026 20:17:09 +0000 Subject: [PATCH 2/2] refactor(cli): propagate JSON write failures from --json mode Address CodeRabbit feedback on #416: `let _ = render_json(...)` in sessions / hotspots / summary swallowed stdout write errors, so a broken pipe or full output destination could exit 0 after a partial write. Plumb the io::Result through emit_json / emit_grouped / render_no_relationships so callers propagate via `?` and the existing `report_error` path turns it into a non-zero exit. Tighten the changelog entry to be impact-first. --- CHANGELOG.md | 6 +-- crates/relayburn-cli/src/commands/hotspots.rs | 6 +-- crates/relayburn-cli/src/commands/sessions.rs | 11 ++++-- crates/relayburn-cli/src/commands/summary.rs | 39 ++++++++++--------- 4 files changed, 34 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5ac55ca..65f7cbc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,10 +6,8 @@ Cross-package release notes for relayburn. Package changelogs contain package-le ### Changed -- `relayburn-cli`: consolidated `format_usd` / `format_int` / `format_tokens` / - `coerce_whole_f64_to_int` duplicates across `overhead`, `compare`, `state`, - `sessions`, `summary`, and `hotspots` onto the canonical helpers in - `render::format` / `render::json`. No user-visible output changes. +- `relayburn-cli`: stdout-write failures in `--json` mode now surface as a + non-zero exit instead of being silently dropped. - `relayburn-sdk`: ledger query verbs (`query_turns`, `query_compactions`, `query_relationships`, `query_tool_result_events`, `query_user_turns`) now push `since` / `until` / `session_id` / `source` / `project` filters diff --git a/crates/relayburn-cli/src/commands/hotspots.rs b/crates/relayburn-cli/src/commands/hotspots.rs index e0c28e2b..c560a041 100644 --- a/crates/relayburn-cli/src/commands/hotspots.rs +++ b/crates/relayburn-cli/src/commands/hotspots.rs @@ -255,7 +255,7 @@ fn run_inner(globals: &GlobalArgs, args: HotspotsArgs) -> anyhow::Result { progress.finish_and_clear(); if globals.json { - emit_json(&result); + emit_json(&result)?; return Ok(0); } let limit = if args.all { usize::MAX } else { DEFAULT_TOP_N }; @@ -263,10 +263,10 @@ fn run_inner(globals: &GlobalArgs, args: HotspotsArgs) -> anyhow::Result { Ok(0) } -fn emit_json(result: &HotspotsResult) { +fn emit_json(result: &HotspotsResult) -> std::io::Result<()> { let mut value = hotspots_result_to_json(result); coerce_whole_f64_to_int(&mut value); - let _ = render_json(&value); + render_json(&value) } fn hotspots_result_to_json(result: &HotspotsResult) -> Value { diff --git a/crates/relayburn-cli/src/commands/sessions.rs b/crates/relayburn-cli/src/commands/sessions.rs index 2215bc70..f5520fad 100644 --- a/crates/relayburn-cli/src/commands/sessions.rs +++ b/crates/relayburn-cli/src/commands/sessions.rs @@ -81,14 +81,19 @@ fn run_list_inner(globals: &GlobalArgs, args: SessionsListArgs) -> anyhow::Resul &since, args.project.as_deref(), args.grep.as_deref(), - ); + )?; } else { emit_human(&result, &since, args.grep.as_deref()); } Ok(0) } -fn emit_json(result: &SessionsListResult, since: &str, project: Option<&str>, grep: Option<&str>) { +fn emit_json( + result: &SessionsListResult, + since: &str, + project: Option<&str>, + grep: Option<&str>, +) -> std::io::Result<()> { let mut filters = json!({ "since": since }); if let Some(p) = project { filters @@ -110,7 +115,7 @@ fn emit_json(result: &SessionsListResult, since: &str, project: Option<&str>, gr "sessions": result.sessions, }); coerce_whole_f64_to_int(&mut payload); - let _ = render_json(&payload); + render_json(&payload) } fn emit_human(result: &SessionsListResult, since: &str, grep: Option<&str>) { diff --git a/crates/relayburn-cli/src/commands/summary.rs b/crates/relayburn-cli/src/commands/summary.rs index a5ccae89..fcecdde7 100644 --- a/crates/relayburn-cli/src/commands/summary.rs +++ b/crates/relayburn-cli/src/commands/summary.rs @@ -269,7 +269,7 @@ fn run_inner(globals: &GlobalArgs, args: SummaryArgs) -> anyhow::Result { match report { SummaryReport::Grouped(report) => { - emit_grouped(globals, &report, &ingest_report); + emit_grouped(globals, &report, &ingest_report)?; } SummaryReport::ByTool(report) => { emit_ingest_prelude(globals, &ingest_report); @@ -392,12 +392,12 @@ fn emit_grouped( globals: &GlobalArgs, report: &SummaryGroupedReport, ingest_report: &relayburn_sdk::IngestReport, -) { +) -> std::io::Result<()> { if globals.json { - emit_json(report, ingest_report); - return; + return emit_json(report, ingest_report); } emit_human(report, ingest_report); + Ok(()) } fn emit_ingest_prelude(globals: &GlobalArgs, ingest_report: &relayburn_sdk::IngestReport) { @@ -424,9 +424,12 @@ fn ingest_prelude_text(ingest_report: &relayburn_sdk::IngestReport) -> String { ) + "\n" } -fn emit_json(report: &SummaryGroupedReport, ingest_report: &relayburn_sdk::IngestReport) { +fn emit_json( + report: &SummaryGroupedReport, + ingest_report: &relayburn_sdk::IngestReport, +) -> std::io::Result<()> { let value = grouped_json_value(report, ingest_report); - let _ = render_json(&value); + render_json(&value) } fn grouped_json_value( @@ -565,7 +568,7 @@ fn render_by_tool_report( ); } coerce_whole_f64_to_int(&mut payload); - let _ = render_json(&payload); + render_json(&payload)?; return Ok(0); } @@ -635,7 +638,7 @@ fn render_subagent_type_report( if globals.json { let mut value = serde_json::to_value(stats)?; coerce_whole_f64_to_int(&mut value); - let _ = render_json(&value); + render_json(&value)?; return Ok(0); } @@ -693,13 +696,13 @@ fn render_relationship_report( return render_relationship_subagent_report(globals, report); } if report.relationships.is_empty() { - return Ok(render_no_relationships(globals)); + return render_no_relationships(globals); } if globals.json { let mut value = json!({ "relationships": report.relationships }); coerce_whole_f64_to_int(&mut value); - let _ = render_json(&value); + render_json(&value)?; return Ok(0); } @@ -748,10 +751,10 @@ fn render_relationship_subagent_report( "message": NO_RELATIONSHIPS_MESSAGE, }); coerce_whole_f64_to_int(&mut value); - let _ = render_json(&value); + render_json(&value)?; return Ok(0); } - return Ok(render_no_relationships(globals)); + return render_no_relationships(globals); } if globals.json { let mut value = json!({ @@ -759,7 +762,7 @@ fn render_relationship_subagent_report( "subagentTypes": report.subagent_types, }); coerce_whole_f64_to_int(&mut value); - let _ = render_json(&value); + render_json(&value)?; return Ok(0); } @@ -796,16 +799,16 @@ fn render_relationship_subagent_report( Ok(0) } -fn render_no_relationships(globals: &GlobalArgs) -> i32 { +fn render_no_relationships(globals: &GlobalArgs) -> anyhow::Result { if globals.json { - let _ = render_json(&json!({ + render_json(&json!({ "relationships": [], "message": NO_RELATIONSHIPS_MESSAGE, - })); + }))?; } else { println!("{NO_RELATIONSHIPS_MESSAGE}"); } - 0 + Ok(0) } fn render_subagent_tree_report( @@ -822,7 +825,7 @@ fn render_subagent_tree_report( "root": root, }); coerce_whole_f64_to_int(&mut value); - let _ = render_json(&value); + render_json(&value)?; return Ok(0); }