Skip to content

fix: results graph clips the wpm peak (#22)#27

Open
hindriix wants to merge 1 commit into
jrnxf:mainfrom
hindriix:upstream-pr/graph-y-axis-fix
Open

fix: results graph clips the wpm peak (#22)#27
hindriix wants to merge 1 commit into
jrnxf:mainfrom
hindriix:upstream-pr/graph-y-axis-fix

Conversation

@hindriix

@hindriix hindriix commented Jul 22, 2026

Copy link
Copy Markdown

What

Fixes the results graph clipping the top of the wpm line (#22).

The bug

The wpm axis upper bound is highest_wpm.round(). When the peak has a fractional part below .5 — e.g. 94.4round() drops the bound to 94, so the peak sits above the axis and gets clipped. The same thing happens to any interior peak, which is exactly why the issue's comment notes it "sometimes happens lower than the max value," not just at the very top.

The log line attached to #22 (...,9.47,94,...) is consistent with this: a sub-.5 peak rounding down.

The fix

Bound the axis with ceil() instead of round() (minimum 1 so the axis never collapses), via a small results_y_max helper so it's unit-testable. The whole line now always fits under the axis.

Verification

cargo test / clippy / fmt --check clean; added a unit test asserting the bound is never below the peak (and handles the zero case).

The wpm axis upper bound was `highest_wpm.round()`. When the peak has a
fraction below 0.5 (e.g. 94.4), round() drops the bound to 94 and the
line is clipped at the top — and the same happens to any interior peak,
which is why it sometimes clips below the max value too (as noted in the
issue's comment).

Bound the axis with `ceil()` instead (via a small `results_y_max` helper,
with a minimum of 1 so the axis never collapses) so the whole line always
fits. Covered by a unit test.
@hindriix
hindriix force-pushed the upstream-pr/graph-y-axis-fix branch from 407334a to d7576c2 Compare July 25, 2026 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant