refactor(distribution): extract + test the "Top X%" maths, and fix a latent "< 0.00%" label#91
Open
Amayyas wants to merge 2 commits into
Open
refactor(distribution): extract + test the "Top X%" maths, and fix a latent "< 0.00%" label#91Amayyas wants to merge 2 commits into
Amayyas wants to merge 2 commits into
Conversation
The distribution panel's standing calculation lived inline in the component, so the one number the app asserts publicly about someone — "TOP 0.01% of GitHub" — was the only piece of statistics in the codebase with no tests. lib/ is where the pure, framework-agnostic logic lives, so it moves there. Behaviour is unchanged: the extracted module was diffed against the old inline version across all 99 ratings on both shipped samples (all accounts and active-only) and the labels are identical, and the rendered panel still reads TOP 0.01% / TOP 0.05% for torvalds in the browser. Tests cover the parts that were easy to get wrong and impossible to notice: the at-or-above count is inclusive of the card's own bucket, a rating below the histogram floor takes the whole sample, and the rule-of-three fallback (nothing observed at or above → the honest claim is "< 3/n", not "top 0%") is marked and labelled with a "<". Plus a monotonicity check over the real histogram and an empty-sample guard, so n = 0 can't produce an Infinity.
When nothing in the sample reaches a card's rating, the panel falls back to the rule-of-three bound, 3/n. That share is formatted with two decimals, so any sample past ~60k accounts rounds it to "0.00" and the card is labelled "Top < 0.00%" — a claim about nobody. Today's sample is n = 18,107, so the bound prints as 0.02% and the bug is invisible. It only surfaces when the histogram is regenerated bigger, which is a documented workflow (scripts/distribution-runner.ts) — so it's the kind of thing that breaks later, in someone else's PR. Floors the displayed percentage at 0.01. Still honest: that number is already an upper bound, so showing "< 0.01%" understates nothing.
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
1 participant
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.
What
The distribution panel's standing calculation lived inline in the component, which made "TOP 0.01% of GitHub" — the one number the app asserts publicly about a person — the only piece of statistics in the codebase with no tests.
lib/is where the pure, framework-agnostic logic lives, so it moves there and gets covered.Two commits, so the fix can be taken or dropped independently of the refactor.
1. Extract + test (no behaviour change)
I verified that rather than asserting it:
torvaldsin a real browser, with no console errors.Tests cover what was easy to get wrong and impossible to notice: the at-or-above count is inclusive of the card's own bucket, a rating below the histogram floor takes the whole sample, and the rule-of-three fallback (nothing observed at or above → the honest claim is
< 3/n, not "top 0%") is marked and labelled with a<. Plus a monotonicity check over the real histogram, and an empty-sample guard son = 0can't produce anInfinity.2. Fix: never label a card "Top < 0.00%"
The rule-of-three share is formatted with two decimals, so any sample past ~60k accounts rounds it to
0.00and the card gets labelledTop < 0.00%— a claim about nobody.Today's sample is
n = 18,107, so the bound prints as0.02%and the bug is invisible right now. It surfaces only when the histogram is regenerated bigger — which is a documented workflow (scripts/distribution-runner.ts), so it's the kind of thing that breaks later, in someone else's PR.The fix floors the displayed percentage at
0.01. Still honest: that number is already an upper bound, so< 0.01%understates nothing. Mutation-checked — reverting the floor turns both new tests red.Result
293 passed(+17), lint clean, build green.