From 9334ac79ff496f347ccad31ad06d93b16c3a264b Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Mon, 17 Aug 2026 14:13:44 -0700 Subject: [PATCH] feat(output): auto-right-align numeric columns in the no-view fallback path A command with no registered view renders through the dynamic column catalog (dynamic_columns), which always built Alignment::Left columns with no way to opt into right-alignment the way a registered view's TableColumn::align can. Detects a field that's a JSON number on every row it appears in (nulls/missing don't disqualify it, but any other type anywhere does) and right-aligns that column automatically. Registered views are unaffected: they still default to Alignment::Left and must opt in explicitly, since a numeric-looking field there (an ID, say) may not be a real measurement. Fixes DEVEX-1020 Co-Authored-By: Claude Sonnet 5 --- cli-engine/docs/concepts.md | 1 + cli-engine/src/output/human.rs | 71 ++++++++++++++++++++++++++- cli-engine/tests/exhaustive_output.rs | 21 ++++++++ cli-engine/tests/foundation.rs | 4 +- 4 files changed, 94 insertions(+), 3 deletions(-) diff --git a/cli-engine/docs/concepts.md b/cli-engine/docs/concepts.md index 03ca4e4..b18eeaa 100644 --- a/cli-engine/docs/concepts.md +++ b/cli-engine/docs/concepts.md @@ -621,6 +621,7 @@ Human output is designed for readable terminal display: bounded by a large pathological-value safety cap) — use it for values that are useless when cut short, such as URLs. - `TableColumn::align(Alignment::Right)` right-aligns a column's header and cells (the default is `Alignment::Left`) — use it for numeric or price columns so digits and decimal points line up instead of looking ragged on the left. Only affects table rendering; a property-bag `key: value` line has no column width to align against. +- A no-view (dynamic) array column auto-right-aligns without any code needed: when a field is a JSON number on every row it appears in (nulls/missing don't count against it, but any string/bool/array/object value anywhere does), that column renders right-aligned, same as if a view had called `.align(Alignment::Right)` on it. This only applies to the dynamic column catalog described above — a registered view's columns always default to `Alignment::Left` and must opt in explicitly, since a view author may have a numeric-looking field (an ID, say) that shouldn't be right-aligned. - `TableColumn::field` supports a dotted path (`"parameters.items"`) to reach a value nested under intermediate objects — useful when a response wraps a list in a pagination/summary envelope. A literal field name containing a `.` is not addressable this way (the `.` is always read as a path separator), matching the same convention `crate::output::fields` already uses for `--fields` projection. - `TableColumn::nested(columns)` opts a column into rendering its value as an indented child table (when the value is a list of objects) or an indented child property bag (when it's a single object), instead of the raw-JSON fallback every other column gets. It's a strict opt-in: a column with no `.nested(...)` renders exactly as before even if its runtime value happens to be list/object shaped. Nesting only applies inside an object's property bag — a row cell inside an array-of-objects table always renders as a single flat value, since a table row is one monospace line and can't itself contain a rendered sub-block. A nested child's own columns may set `.nested(...)` again for a grandchild table or property bag; the width budget and hide-before-truncate behavior below apply to every nesting level, narrowed by two spaces of indent per level. - When the terminal is too narrow for every column, hiding a column is diff --git a/cli-engine/src/output/human.rs b/cli-engine/src/output/human.rs index a15c7de..bfc27f0 100644 --- a/cli-engine/src/output/human.rs +++ b/cli-engine/src/output/human.rs @@ -492,6 +492,23 @@ fn dynamic_columns(fields: &str, natural_keys: impl FnOnce() -> Vec) -> .collect() } +/// True when at least one item has a JSON number at `field`, and no item +/// with a present, non-null value at `field` holds anything else. +fn column_is_all_numeric(items: &[Value], field: &str) -> bool { + let mut saw_number = false; + for item in items { + match item + .as_object() + .and_then(|map| resolve_field_path(map, field)) + { + Some(Value::Number(_)) => saw_number = true, + Some(Value::Null) | None => {} + Some(_) => return false, + } + } + saw_number +} + /// Appends footer hints for truncated cells and/or hidden columns to `out` /// (a no-op when neither happened). Mirrors `append_next_actions`: writes /// directly into `out` rather than building a separate string. @@ -953,7 +970,16 @@ fn render_array( if !items.iter().all(Value::is_object) { return (render_array_lines(items), RenderNotes::default()); } - let columns = dynamic_columns(fields, || first_map.keys().cloned().collect()); + let columns: Vec = dynamic_columns(fields, || first_map.keys().cloned().collect()) + .into_iter() + .map(|column| { + if column_is_all_numeric(items, &column.field) { + column.align(Alignment::Right) + } else { + column + } + }) + .collect(); render_array_with_columns(items, &columns, available_width, pagination) } @@ -1610,6 +1636,49 @@ mod tests { ); } + #[test] + fn no_view_array_rendering_right_aligns_a_column_that_is_numeric_on_every_row() { + let items = vec![ + json!({ "name": "small", "count": 3 }), + json!({ "name": "bigger", "count": 42 }), + ]; + + let (out, _notes) = render_array(&items, "name,count", 80, None); + let mut lines = out.lines(); + let header_line = lines.next().expect("header line"); + let row_lines: Vec<&str> = lines.skip(1).take(2).collect(); + + assert!(header_line.ends_with(" COUNT"), "{header_line}"); + assert!(row_lines[0].ends_with(" 3"), "{}", row_lines[0]); + assert!(row_lines[1].ends_with(" 42"), "{}", row_lines[1]); + assert!(header_line.starts_with("NAME "), "{header_line}"); + } + + #[test] + fn no_view_array_rendering_keeps_a_mixed_type_column_left_aligned() { + // Same field is a number on one row and a string on another — a + // single non-number value anywhere disqualifies the whole column, + // matching how right-aligning it would look ragged next to text. + let items = vec![json!({ "code": 1 }), json!({ "code": "default" })]; + + let (out, _notes) = render_array(&items, "", 80, None); + let header_line = out.lines().next().expect("header line"); + + assert!(header_line.starts_with("CODE"), "{header_line}"); + } + + #[test] + fn no_view_array_rendering_keeps_an_all_null_column_left_aligned() { + // No row ever has a number at this field, so there's no positive + // signal to right-align on. + let items = vec![json!({ "note": null }), json!({ "note": null })]; + + let (out, _notes) = render_array(&items, "", 80, None); + let header_line = out.lines().next().expect("header line"); + + assert!(header_line.starts_with("NOTE"), "{header_line}"); + } + #[test] fn no_view_array_rendering_follows_requested_field_order() { // Reproduces the real-world `domain suggest` symptom: a command with diff --git a/cli-engine/tests/exhaustive_output.rs b/cli-engine/tests/exhaustive_output.rs index 3092462..154051b 100644 --- a/cli-engine/tests/exhaustive_output.rs +++ b/cli-engine/tests/exhaustive_output.rs @@ -264,6 +264,27 @@ fn human_view_right_aligned_column_lines_up_prices_in_table_output() { ); } +#[test] +fn human_view_with_no_registered_view_auto_right_aligns_a_numeric_column() { + // No TableColumn list at all — this is the fallback/dynamic-column path + // a command falls into when it never calls `.with_view(...)`. A field + // that's a JSON number on every row (here, an endpoint count) should + // still line up on the right, matching what an explicit view would get + // from `.align(Alignment::Right)`. + let envelope = Envelope::success( + json!([ + {"domain": "commerce", "endpoints": 3}, + {"domain": "domains", "endpoints": 42} + ]), + "api:domain:list", + ); + + assert_eq!( + render_human_with_view(&envelope, None, "domain,endpoints"), + "DOMAIN ENDPOINTS\n-------- ---------\ncommerce 3\ndomains 42\n\n(2 rows)\n" + ); +} + #[test] fn global_registries_tolerate_repeated_and_concurrent_registration() { let prefix = format!( diff --git a/cli-engine/tests/foundation.rs b/cli-engine/tests/foundation.rs index 8a8821e..fb5077f 100644 --- a/cli-engine/tests/foundation.rs +++ b/cli-engine/tests/foundation.rs @@ -9148,7 +9148,7 @@ fn human_renderer_preserves_json_number_text() { assert_eq!( rendered, - "NAME RATIO SCORE\n----- ----- -----\nalpha 1.0 1.25 \n\n(1 rows)\n" + "NAME RATIO SCORE\n----- ----- -----\nalpha 1.0 1.25\n\n(1 rows)\n" ); } @@ -9164,7 +9164,7 @@ fn human_renderer_formats_non_integer_json_floats_with_serde_json_text() { assert_eq!( render(OutputFormat::Human, &envelope).expect("floats should render"), - "NAME SCORE \n----- ---------\nlarge 1000000.5\nsmall 1.2345e-7\n\n(2 rows)\n" + "NAME SCORE\n----- ---------\nlarge 1000000.5\nsmall 1.2345e-7\n\n(2 rows)\n" ); }