From c2ad64568e2979fc0a924f5755e0d9e3717f0303 Mon Sep 17 00:00:00 2001 From: Vyncint Ng Date: Mon, 7 Sep 2026 18:48:37 +0700 Subject: [PATCH] fix(tui): round Describe's percentiles instead of clipping a digit off MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `approx_percentile_cont` renders full precision, and Describe's columns are six characters wide. Ratatui clips a cell that overflows, so the 2M-row demo table's P99 of `id` — 1979969.2416513609 — printed as `197999`: ten times too small, below the median displayed in the same row, and with nothing to show a digit had been dropped. Found while exercising 0.1.3 from the Homebrew build: before id Int64 0 1999999 0 512965 996995 197999 after id Int64 0 1999999 0 500678 989902 1.98e6 A cell that already fits is now left exactly as the query rendered it, so `0.0` and `24.75` are untouched and the existing snapshots do not move. Anything longer is rounded to the most decimals that fit and falls back to an exponent form when even the integer part is too wide. Over-long text is marked with an ellipsis (`FixedSi…`) rather than cut silently, and the widths now live in one `DESCRIBE_WIDTHS` constant that the cell formatters and the layout both read — previously the widths existed only in the layout, so no formatter could know what had to fit. Tested: a unit test asserts the three real percentile strings render in order and within 1% of their values, which fails with `percentiles out of order: 505700 996257 197996` against the old clipping; a property test holds every cell inside every width from 1 to 8 for negatives, subnormals, 1e300 and non-numeric input; and a frame-level test renders the panel through `TestBackend` and parses the `id` row back to check P99 lands above P50. Verified against a real 2M-row table through a PTY. Not done: the panel is still fixed-width and drops columns on a narrow terminal, and min/max for a float column can still round — both are existing behaviour, not the truncation this fixes. Signed-off-by: Vyncint Ng --- CHANGELOG.md | 13 ++ crates/oxidelake-tui/src/render.rs | 167 ++++++++++++++++-- crates/oxidelake-tui/tests/tui_render_test.rs | 68 ++++++- 3 files changed, 227 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4db62e9..5a7a0fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,19 @@ versions (0.x) may contain breaking changes; they are always listed under a ## [Unreleased] +### Fixed + +- **The dashboard's Describe panel no longer prints a truncated percentile as + if it were the value.** `approx_percentile_cont` renders full precision, and + the panel's six-character columns were clipped from the right, so the 2M-row + demo table's P99 of `id` (1979969.24) displayed as `197999` — ten times too + small and below the median in the same row. Numeric cells that do not fit + are now rounded to the most decimals that fit, falling back to an exponent + form (`1.98e6`); a value that already fits is left exactly as the query + rendered it. Over-long text cells are marked with an ellipsis rather than + cut silently, and the column widths now live in one constant the cell + formatters and the layout share. + ## [0.1.3] - 2026-09-07 Release-build changes only; no crate code changed since 0.1.1. This is the diff --git a/crates/oxidelake-tui/src/render.rs b/crates/oxidelake-tui/src/render.rs index 45993c5..ebd5d44 100644 --- a/crates/oxidelake-tui/src/render.rs +++ b/crates/oxidelake-tui/src/render.rs @@ -243,6 +243,67 @@ fn render_telemetry(state: &AppState, model: &DashboardModel, frame: &mut Frame< ); } +/// Describe's column widths, in header order. Named because the cell +/// formatters below have to agree with the layout: ratatui clips a cell that +/// overflows its column, and for a number that silently changes its value. +const DESCRIBE_WIDTHS: [u16; 8] = [8, 8, 7, 7, 7, 6, 6, 6]; + +/// Drops a decimal fraction's trailing zeros, and the point left behind. +fn trim_zeros(rendered: &str) -> String { + if !rendered.contains('.') { + return rendered.to_owned(); + } + rendered + .trim_end_matches('0') + .trim_end_matches('.') + .to_owned() +} + +/// Text that cannot fit, marked as shortened rather than quietly cut. +fn fit_text(raw: &str, width: usize) -> String { + if raw.chars().count() <= width { + return raw.to_owned(); + } + match width { + 0 => String::new(), + _ => raw.chars().take(width - 1).chain(['…']).collect(), + } +} + +/// Fits a rendered number into `width` columns without ever cutting a digit. +/// +/// `approx_percentile_cont` renders full precision — the 2M-row demo table's +/// P99 of `id` arrives as `1979969.2416513609`. Clipped to the six columns +/// Describe has, that printed `197999`: an order of magnitude low, and below +/// the median displayed above it, with nothing to show it had been cut. So a +/// value that already fits is kept exactly as the query rendered it, and +/// anything longer is *rounded* to the most decimals that fit, falling back to +/// an exponent form when even the integer part is too wide. +fn fit_number(raw: &str, width: usize) -> String { + if raw.chars().count() <= width { + return raw.to_owned(); + } + let Ok(value) = raw.parse::() else { + return fit_text(raw, width); + }; + if !value.is_finite() { + return fit_text(raw, width); + } + for precision in (0..=3).rev() { + let candidate = trim_zeros(&format!("{value:.precision$}")); + if candidate.chars().count() <= width { + return candidate; + } + } + for precision in (0..=2).rev() { + let candidate = format!("{value:.precision$e}"); + if candidate.chars().count() <= width { + return candidate; + } + } + fit_text(raw, width) +} + fn render_describe(state: &AppState, model: &DashboardModel, frame: &mut Frame<'_>, area: Rect) { let header = Row::new(["column", "type", "min", "max", "nulls", "p25", "p50", "p99"].map(Cell::from)) @@ -251,28 +312,20 @@ fn render_describe(state: &AppState, model: &DashboardModel, frame: &mut Frame<' .profiles .iter() .map(|p| { + let w = DESCRIBE_WIDTHS.map(usize::from); Row::new(vec![ - Cell::from(p.name.clone()), - Cell::from(p.data_type.clone()), - Cell::from(p.min.clone()), - Cell::from(p.max.clone()), - Cell::from(p.null_count.to_string()), - Cell::from(p.p25.clone()), - Cell::from(p.p50.clone()), - Cell::from(p.p99.clone()), + Cell::from(fit_text(&p.name, w[0])), + Cell::from(fit_text(&p.data_type, w[1])), + Cell::from(fit_number(&p.min, w[2])), + Cell::from(fit_number(&p.max, w[3])), + Cell::from(fit_number(&p.null_count.to_string(), w[4])), + Cell::from(fit_number(&p.p25, w[5])), + Cell::from(fit_number(&p.p50, w[6])), + Cell::from(fit_number(&p.p99, w[7])), ]) }) .collect(); - let widths = [ - Constraint::Length(8), - Constraint::Length(8), - Constraint::Length(7), - Constraint::Length(7), - Constraint::Length(7), - Constraint::Length(6), - Constraint::Length(6), - Constraint::Length(6), - ]; + let widths = DESCRIBE_WIDTHS.map(Constraint::Length); frame.render_widget( Table::new(rows, widths).header(header).block(panel_block( "Describe", @@ -295,6 +348,84 @@ mod tests { assert_eq!(human_bytes(6 * 1024 * 1024 * 1024), "6.0 GiB"); } + /// A percentile parses back to the number it stands for. + fn num(rendered: &str) -> f64 { + rendered.parse().unwrap_or(f64::NAN) + } + + #[test] + fn a_value_that_fits_is_left_exactly_as_the_query_rendered_it() { + for raw in ["0", "0.0", "99", "24.75", "1999999", "", "s0"] { + assert_eq!(fit_number(raw, 7), raw); + } + assert_eq!(fit_text("Int64", 8), "Int64"); + } + + /// The bug: `id`'s P99 on the 2M-row demo table arrives as + /// `1979969.2416513609`, and six columns of hard clipping printed + /// `197999` — ten times too small, and below the median above it. + #[test] + fn wide_percentiles_round_instead_of_losing_a_digit() { + let (p25, p50, p99) = ( + fit_number("505700.81345880596", 6), + fit_number("996257.6842335836", 6), + fit_number("1979969.2416513609", 6), + ); + assert_ne!(p99, "197999", "P99 was clipped mid-integer"); + assert!( + num(&p25) < num(&p50) && num(&p50) < num(&p99), + "percentiles out of order: {p25} {p50} {p99}" + ); + for (rendered, want) in [(&p25, 505_700.81), (&p50, 996_257.68), (&p99, 1_979_969.24)] { + let error = (num(rendered) - want).abs() / want; + assert!(error < 0.01, "{rendered} is not within 1% of {want}"); + } + } + + #[test] + fn no_cell_can_overflow_its_column() { + let samples = [ + "1979969.2416513609", + "-1979969.2416513609", + "0.000012345678", + "123456789012345", + "1e300", + "-1e-300", + "6.122512376708984", + "not a number at all", + "", + ]; + for raw in samples { + for width in 1..=8 { + let cell = fit_number(raw, width); + assert!( + cell.chars().count() <= width, + "{raw:?} rendered {cell:?}, wider than {width}" + ); + } + } + } + + #[test] + fn text_too_long_is_marked_as_shortened() { + assert_eq!(fit_text("FixedSizeList(8 x Float32)", 8), "FixedSi…"); + assert_eq!(fit_text("Float64", 1), "…"); + assert_eq!(fit_text("abc", 0), ""); + } + + #[test] + fn trailing_zeros_go_but_the_value_stays() { + assert_eq!(trim_zeros("24.750"), "24.75"); + assert_eq!(trim_zeros("0.000"), "0"); + assert_eq!(trim_zeros("996258"), "996258"); + assert_eq!(trim_zeros("100"), "100"); + } + + #[test] + fn every_describe_column_has_a_width() { + assert_eq!(DESCRIBE_WIDTHS.len(), 8, "one width per Describe column"); + } + #[test] fn ratio_is_clamped() { assert_eq!(ratio(1, 0), 0.0); diff --git a/crates/oxidelake-tui/tests/tui_render_test.rs b/crates/oxidelake-tui/tests/tui_render_test.rs index 8074610..dc0d221 100644 --- a/crates/oxidelake-tui/tests/tui_render_test.rs +++ b/crates/oxidelake-tui/tests/tui_render_test.rs @@ -4,17 +4,29 @@ #![allow(clippy::unwrap_used, clippy::expect_used)] -use oxidelake_tui::{AppState, KeyInput, Panel, Transition, demo_model, render}; +use oxidelake_tui::DashboardModel; +use oxidelake_tui::{AppState, ColumnProfile, KeyInput, Panel, Transition, demo_model, render}; use ratatui::Terminal; use ratatui::backend::TestBackend; fn frame(width: u16, height: u16, state: &AppState) -> String { + frame_of(width, height, state, &demo_model()) +} + +fn frame_of(width: u16, height: u16, state: &AppState, model: &DashboardModel) -> String { let mut terminal = Terminal::new(TestBackend::new(width, height)).unwrap(); - let model = demo_model(); - terminal.draw(|f| render(state, &model, f)).unwrap(); + terminal.draw(|f| render(state, model, f)).unwrap(); terminal.backend().to_string() } +/// The row of a rendered frame that profiles `column`. +fn describe_row<'a>(frame: &'a str, column: &str) -> &'a str { + frame + .lines() + .find(|line| line.contains(&format!("{column} ")) && line.contains("Int64")) + .unwrap_or_else(|| panic!("no Describe row for {column}:\n{frame}")) +} + #[test] fn initial_frame_80x24() { let text = frame(80, 24, &AppState::new(3)); @@ -74,3 +86,53 @@ fn rendering_is_deterministic_and_stable_across_sizes() { } } } + +/// Describe renders what `approx_percentile_cont` actually returns: full +/// precision. The 2M-row demo table's P99 of `id` is `1979969.2416513609`, +/// and clipping it to the column's six characters printed `197999` — below +/// the median in the same row, with nothing to mark the cut. +#[test] +fn wide_percentiles_render_in_order_in_the_panel() { + let mut model = demo_model(); + model.profiles = vec![ColumnProfile { + name: "id".into(), + data_type: "Int64".into(), + min: "0".into(), + max: "1999999".into(), + null_count: 0, + p25: "505700.81345880596".into(), + p50: "996257.6842335836".into(), + p99: "1979969.2416513609".into(), + }]; + let text = frame_of(160, 40, &AppState::new(3), &model); + let row = describe_row(&text, "id"); + assert!( + !row.contains("197999 ") && !row.contains("197999|"), + "P99 was clipped mid-integer: {row}" + ); + let numbers: Vec = row + .split_whitespace() + .filter_map(|cell| cell.parse::().ok()) + .collect(); + let (p25, p50, p99) = (505_700.81, 996_257.68, 1_979_969.24); + for want in [p25, p50, p99] { + assert!( + numbers.iter().any(|got| (got - want).abs() / want < 0.01), + "no cell within 1% of {want} in: {row}\nparsed {numbers:?}" + ); + } + let p99_cell = numbers + .iter() + .copied() + .find(|got| (got - p99).abs() / p99 < 0.01) + .unwrap_or(f64::NAN); + let p50_cell = numbers + .iter() + .copied() + .find(|got| (got - p50).abs() / p50 < 0.01) + .unwrap_or(f64::NAN); + assert!( + p99_cell > p50_cell, + "P99 {p99_cell} not above P50 {p50_cell}" + ); +}