Fix flaky randomString bias test - #993
Merged
Merged
Conversation
Contributor
|
The changes in this PR will be included in the next version bump. Bump files in this PRClick here if you want to add another bump file to this PR This comment is maintained by bumpy. |
Contributor
📦 Bundle sizeno change to bundle size
dist/ only; native binaries are versioned separately and not counted here. |
Contributor
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes across the test-only update to randomString() bias coverage and its release metadata.
- Targeted bias assertion: Replaces noisy per-character bounds with an aggregate check over the eight default-charset indices that naive modulo sampling overrepresents.
- Sanity coverage: Retains a low-risk assertion that every default-charset character appears in the 10,000-character sample.
- Release metadata: Records the test-only change as a no-bump entry for
varlock.
azure/gpt-5.6-sol | 𝕏
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.


The unbiased-output test for randomString() checked every charset char against a +/-30% bound. At n=10000 that bound sits at ~3.8 sigma, and with 62 chars checked on both tails it fails by pure chance roughly once every 130 runs (seen in CI: 210 vs 209.68). The old assertion also could not catch the modulo bias it guards against: a naive byte % 62 puts over-represented chars at ~195 counts, under the old 209.7 ceiling.
Replaced it with an aggregate count of the first 8 charset chars, exactly the ones modulo bias inflates. Unbiased sampling lands around 1290 (stddev ~33.5), biased around 1562, so the 1425 threshold is ~4 sigma from both: false failures drop to ~1 in 30k runs while a simulated biased sampler trips it in 20/20 trials. Kept a loose sanity check that every charset char appears at least once.
No resolver changes; the implementation's rejection sampling was already correct.