fix(tui): round Describe's percentiles instead of clipping a digit off - #18
Merged
Conversation
`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 <vyncint@icloud.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Today
Found while exercising the 0.1.3 Homebrew build against a 2M-row table. The dashboard's Describe panel showed this:
That P99 is below the P50 on the same row. The true value is 1,979,969.24.
approx_percentile_contrenders full precision (1979969.2416513609) and the percentile columns are six characters wide, so ratatui clipped the cell from the right and the cut landed mid-integer. Nothing marked it. P25 and P50 were being truncated too, but their integer parts happen to be six digits, so they looked plausible.Change
0.0and24.75are untouched and the existing snapshots do not move.FixedSi…forFixedSizeList(8 x non-null Float32)) instead of being cut silently.DESCRIBE_WIDTHSconstant that the cell formatters and the layout both read. Previously they existed only in the layout array, which is why no formatter could know what had to fit — that is the structural cause.Tests
Three, at three levels:
wide_percentiles_round_instead_of_losing_a_digitfeeds the three real percentile strings and asserts they render in order and within 1% of their values. Against the old clipping it fails withpercentiles out of order: 505700 996257 197996.no_cell_can_overflow_its_columnholds every cell inside every width from 1 to 8 across negatives, subnormals,1e300and non-numeric input.wide_percentiles_render_in_order_in_the_panelrenders the real panel throughTestBackend, finds theidrow and parses its cells back to check P99 lands above P50.Also verified by driving the built binary through a PTY against a real 2M-row table, which is where the after-row above comes from. Full workspace tests,
cargo fmt --checkand clippy-D warningson the crate are green; the PTY suite and all insta snapshots pass unchanged.Not done
min/maxfor a float column can still round when very long.Both are pre-existing behaviour rather than the truncation this fixes.