-
-
Notifications
You must be signed in to change notification settings - Fork 0
fix: bad logic, add tests for every module #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
nberlette
commented
Jan 14, 2026
- tests(cbrt): add tests in cbrt.test.ts
- tests(log2): add tests in log2.test.ts
- tests(log10): add tests in log10.test.ts
- tests(trunc): add tests in trunc.test.ts
- tests(ceil): add tests in ceil.test.ts
- tests(asin): add tests in asin.test.ts
- tests(random): add tests in random.test.ts
- tests(log): add tests in log.test.ts
- tests(float32): add tests in float32/guards.test.ts
- tests(float32): add tests in float32/constants.test.ts
- tests(float32): add tests in float32/index.test.ts
- tests(float32): add tests in float32/encode.test.ts
- tests(float32): add tests in float32/decode.test.ts
- tests(float32): add tests in float32/round.test.ts
- tests(clamp): add tests in clamp.test.ts
- tests(clz32): add tests in clz32.test.ts
- tests(abs): add tests in abs.test.ts
- tests(constants): add tests in constants/log2e.test.ts
- tests(constants): add tests in constants/max_safe_integer.test.ts
- tests(constants): add tests in constants/ln2.test.ts
- tests(constants): add tests in constants/log10e.test.ts
- tests(constants): add tests in constants/positive_zero.test.ts
- tests(constants): add tests in constants/epsilon.test.ts
- tests(constants): add tests in constants/negative_zero.test.ts
- tests(constants): add tests in constants/index.test.ts
- tests(constants): add tests in constants/nan.test.ts
- tests(constants): add tests in constants/sqrt1_2.test.ts
- tests(constants): add tests in constants/ln10.test.ts
- tests(constants): add tests in constants/positive_infinity.test.ts
- tests(constants): add tests in constants/min_safe_integer.test.ts
- tests(constants): add tests in constants/e.test.ts
- tests(constants): add tests in constants/max_value.test.ts
- tests(constants): add tests in constants/negative_infinity.test.ts
- tests(constants): add tests in constants/pi.test.ts
- tests(constants): add tests in constants/sqrt2.test.ts
- tests(constants): add tests in constants/infinity.test.ts
- tests(constants): add tests in constants/min_value.test.ts
- tests(expm1): add tests in expm1.test.ts
- tests(sin): add tests in sin.test.ts
- tests(constants): add tests in constants.test.ts
- tests(atan): add tests in atan.test.ts
- tests(max): add tests in max.test.ts
- tests(asinh): add tests in asinh.test.ts
- tests(sinh): add tests in sinh.test.ts
- tests(index): add tests in index.test.ts
- tests(acos): add tests in acos.test.ts
- tests(atan2): add tests in atan2.test.ts
- tests(guards): add tests in guards/positive_zero.test.ts
- tests(guards): add tests in guards/safe_integer.test.ts
- tests(guards): add tests in guards/negative_zero.test.ts
- tests(guards): add tests in guards/index.test.ts
- tests(guards): add tests in guards/nan.test.ts
- tests(guards): add tests in guards/integer.test.ts
- tests(guards): add tests in guards/positive_infinity.test.ts
- tests(guards): add tests in guards/negative_infinity.test.ts
- tests(guards): add tests in guards/infinity.test.ts
- tests(guards): add tests in guards/finite.test.ts
- tests(tan): add tests in tan.test.ts
- tests(ieee754): add tests in ieee754.test.ts
- tests(floor): add tests in floor.test.ts
- tests(tanh): add tests in tanh.test.ts
- tests(imul): add tests in imul.test.ts
- tests(float16): add tests in float16/guards.test.ts
- tests(float16): add tests in float16/constants.test.ts
- tests(float16): add tests in float16/index.test.ts
- tests(float16): add tests in float16/encode.test.ts
- tests(float16): add tests in float16/decode.test.ts
- tests(float16): add tests in float16/round.test.ts
- tests(types): add tests in types/precision.test.ts
- tests(types): add tests in types/safe_integer.test.ts
- tests(types): add tests in types/index.test.ts
- tests(types): add tests in types/integer.test.ts
- tests(types): add tests in types/finite.test.ts
- tests(types): add tests in types/float.test.ts
- tests(cos): add tests in cos.test.ts
- tests(sqrt): add tests in sqrt.test.ts
- tests(atanh): add tests in atanh.test.ts
- tests(min): add tests in min.test.ts
- tests(pow): add tests in pow.test.ts
- tests(trigonometry): add tests in trigonometry.test.ts
- tests(sign): add tests in sign.test.ts
- tests(exp): add tests in exp.test.ts
- tests(f16round): add tests in f16round.test.ts
- tests(log1p): add tests in log1p.test.ts
- tests(round): add tests in round.test.ts
- tests(hypot): add tests in hypot.test.ts
- tests(cosh): add tests in cosh.test.ts
- tests(fround): add tests in fround.test.ts
- tests(acosh): add tests in acosh.test.ts
- tests(internal): create shared test helpers module
- chore(internal): add ArrayFrom to primordials
- refactor(random): clean up MT19937 class, use ArrayFrom primordial
- fix(float16,float32): use type-only imports for FloatFormat
- fix(ieee754): ensure floating point rounding logic is sound
- chore: update license
- docs(readme): improve doctests, use node:assert
- config: rewrite CI workflow for npm trusted publishing
- fix(constants): ensure negative-zero module is exported
- tests: fix doctest error in src/index.ts
- config: update deno.json tasks and settings
- fix(abs): ensure abs behaves like Math.abs
- fix(asinh): ensure asinh behaves like Math.asinh
- fix(atan): ensure atan behaves like Math.atan
- fix(atan2): ensure atan2 behaves like Math.atan2
- fix(cbrt): ensure cbrt behaves like Math.cbrt
- fix(ceil): ensure ceil behaves like Math.ceil
- fix(clz32): ensure clz32 behaves like Math.clz32
- fix(floor): ensure floor behaves like Math.floor
- fix(hypot): ensure hypot behaves like Math.hypot
- fix(min): ensure min behaves like Math.min
- fix(round): ensure round behaves like Math.round
- fix(sinh): ensure sinh behaves like Math.sinh
- fix(sqrt): ensure sqrt behaves like Math.sqrt
- fix(trunc): ensure trunc behaves like Math.trunc
- refactor(float16,float32)!: remove constants re-exports
- ci: add build script
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds comprehensive test coverage for every module in the math library (100+ new test files), fixes critical bugs related to signed zero handling and floating-point edge cases, refactors the Mersenne Twister implementation, and introduces build tooling with an npm publish workflow.
Changes:
- Adds test files for all math functions, constants, guards, types, and float16/32 modules
- Fixes bugs in 15+ math functions (abs, asinh, atan, atan2, cbrt, ceil, clz32, floor, hypot, min, round, sinh, sqrt, trunc) to properly handle signed zero, infinity, and NaN edge cases
- Refactors MT19937 class to use module-level constants and ArrayFrom primordial
- Changes float16/float32 imports to type-only imports for FloatFormat
- Adds build script for npm publishing and rewrites CI workflow
Reviewed changes
Copilot reviewed 118 out of 118 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/internal/_test_utils.ts | New test helper utilities (assertSameValue, assertClose) |
| src/internal/ieee754.ts | Improved floating-point rounding logic with overflow handling |
| src/internal/primordials.ts | Added ArrayFrom primordial binding |
| src/abs.ts | Fixed to return +0 for both +0 and -0 inputs |
| src/asinh.ts, src/cbrt.ts, src/sinh.ts | Added signed zero preservation |
| src/atan.ts | Fixed Taylor series calculation errors |
| src/atan2.ts | Comprehensive rewrite for correct signed zero handling |
| src/ceil.ts, src/floor.ts | Rewrote to eliminate circular dependencies |
| src/clz32.ts | Replaced logarithm-based implementation with bit manipulation |
| src/hypot.ts | Fixed NaN/infinity handling, removed max() dependency |
| src/min.ts | Added signed zero preference handling |
| src/round.ts, src/trunc.ts | Rewrote for correct signed zero behavior |
| src/sqrt.ts | Added explicit infinity handling |
| src/random.ts | Refactored MT19937 to use constants at module level |
| src/float16/constants.ts, src/float32/constants.ts | Changed to type-only imports |
| src/float16/index.ts, src/float32/index.ts | Removed constants re-exports (breaking change) |
| src/constants/index.ts | Fixed missing negative_zero export |
| All test files (90+) | Comprehensive test coverage added |
| scripts/build.ts | New build script for dnt-based npm publishing |
| .github/workflows/ci.yml | Rewritten CI with JSR/NPM/GPR publishing support |
| LICENSE | Updated copyright year and formatting |
| README.md | Updated documentation examples to use node:assert |
| deno.json | Added tasks, lock file, nodeModulesDir config |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // deno-coverage-ignore-start | ||
| return y < 0 || isNegativeZero(y) ? -PI : PI; | ||
| } else if (isFinite(x) && !isFinite(y)) { | ||
| return y > 0 ? PI_OVER_2 : -PI_OVER_2; | ||
| } else if (!isFinite(x) && isFinite(y)) { | ||
| // preserve the sign of y for -0 | ||
| if (x > 0) return isNegativeZero(y) || y < 0 ? -0 : 0; | ||
| return y >= 0 && !isNegativeZero(y) ? PI : -PI; | ||
| return y < 0 || isNegativeZero(y) ? -PI : PI; | ||
| } | ||
| // deno-coverage-ignore-stop |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the atan2 function, lines 49-50 and 56 have coverage ignore comments. The logic at line 50 appears unreachable - if x is negative infinity and y is finite, the condition at line 51 would be true (since !isFinite(y) is false), so the code would fall through to line 55. This suggests the logic flow may need review.
| @@ -0,0 +1,394 @@ | |||
| #!/usr/bin/env -S deno run -A | |||
| // deno-lint-ignore-file no-explicit-any | |||
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment at line 23 states "deno-lint-ignore-file no-explicit-any" but this is a build script that will be run with Deno's linter. Consider whether ignoring all explicit any usages in this file is appropriate, or if specific locations should be marked instead for better type safety.
| @@ -1,7 +1,6 @@ | |||
| The MIT License (MIT) | |||
| # The MIT License (MIT) | |||
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The LICENSE file header changed from "The MIT License (MIT)" to "# The MIT License (MIT)" (adding a markdown heading). This may cause issues with automated license detection tools that expect the standard MIT License format. Consider keeping the original format without the markdown heading symbol.
| # The MIT License (MIT) | |
| The MIT License (MIT) |
| const cutoff = pow(2, normal); | ||
|
|
||
| const sign = value < 0 ? 1n : 0n; | ||
| const sign = BigInt(value < 0 ? 1 : 0); |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using BigInt(value < 0 ? 1 : 0) is more verbose than the original value < 0 ? 1n : 0n. The original pattern using bigint literals is clearer and more idiomatic for bigint operations.
| if (mant_bits === mant_scale) { | ||
| mant_bits = 0; | ||
| expo++; | ||
| } |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition in line 110 checks if mant_bits === mant_scale, which would mean the mantissa has overflowed. However, this should trigger rounding up the exponent and resetting mantissa to 0. Consider whether this logic correctly handles the edge case where incrementing the exponent causes it to overflow into infinity territory - the check at line 116 should catch this, but verify the flow is correct.
| @@ -1,5 +1,6 @@ | |||
| import { describe, it } from "jsr:@std/testing@1/bdd"; | |||
| import { expect } from "jsr:@std/expect@1"; | |||
| import assert from "node:assert"; | |||
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused import assert.