Pr84 #85
Merged
Pr84 #85
Conversation
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
* test: unify web and VM test expectations Remove platform-specific test expectations in preparation for cross-platform fixes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: implement cross-platform half-precision float encoding The ieee754 package's toFloat16Bytes() uses ByteData.setInt16() which behaves incorrectly on JS where bitwise operators are limited to 32-bit signed integers. This implementation constructs bytes directly. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: handle Infinity encoding by checking special doubles first On JS, double.infinity and double.nan might incorrectly match the int type check, leading to BigInt.from(infinity) which throws. This fix checks for NaN/Infinity before the int type check. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: use explicit safe integer bounds for BigInt handling on JS BigInt.isValidInt is unreliable on JS for values > 2^53-1. This fix adds explicit bitLength checks to ensure safe integer handling across all platforms. - arg.dart: operator ~() now returns ArgBigInt when result exceeds JS safe integer range - int.dart: CborInt factory uses explicit safe integer check Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: add web platform support for Uint64/Int64 typed arrays ByteData.getUint64()/setUint64() don't exist on JS/WASM. This fix: - Adds platform detection constants for JS and WASM - Implements 64-bit integer read/write using two 32-bit operations - Uses multiplication instead of bit shifts to avoid JS limitations - Properly handles signed integers via two's complement conversion Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: update Uint64 test to work cross-platform Remove Uint64List type expectation since web platforms return List<int> instead. The value check still ensures correctness. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: implement cross-platform half-precision float decoding The ieee754 package's fromFloat16Bytes() uses ByteData.getInt16() which interprets bytes as a signed integer on JS. For values with the high bit set (negative numbers, large exponents), this produces incorrect results. For example, -4.0 (bytes 0xC400) was decoded as -33554428 instead of -4.0. This fix: - Adds fromFloat16Bytes() to custom float16.dart utility - Uses only unsigned byte operations that work consistently across platforms - Updates all decoder call sites to use the custom implementation - Documents the rationale in the utility file Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: replace ieee754 dependency with custom float_utils Remove ieee754 package dependency entirely and implement custom float encoding/decoding utilities that work correctly across all platforms (VM, dart2js, dart2wasm). Key changes: - Rename float16.dart to float_utils.dart and add float32/float64 helpers - Add isFloat16Lossless, isFloat32Lossless, isFloat64Lossless functions - Add toFloat32Bytes, toFloat64Bytes, fromFloat32Bytes, fromFloat64Bytes - Update double.dart, stage3.dart, pretty_print.dart to use float_utils - Fix negative zero encoding in CborSmallInt (check value == 0 first) - Use `identical(0, 0.0)` for reliable JS platform detection - Update tests with platform-specific expectations for integer-valued doubles that cannot be distinguished from integers on JS This fixes remaining JS test failures by properly detecting and handling: - Large doubles exceeding safe integer range (now encoded as floats) - Negative zero in integer encoding (now correctly encodes as 0) - JSON encoding of integer-valued doubles on JS All 423 VM tests, 411 JS tests, and 411 WASM tests now pass. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Removed ieee dep * [chore] auto-formatted code * test: add exhaustive tests for float_utils.dart Add comprehensive test coverage for the custom float encoding/decoding utilities that replaced the ieee754 package. Tests cover: - Float16 (half-precision): encoding, decoding, round-trips - Special values: NaN, +/-Infinity, +/-0.0 - Normal and subnormal values - Overflow to infinity, underflow to zero - RFC 8949 Appendix A test vectors - Float32 (single-precision): encoding, decoding, round-trips - Float64 (double-precision): encoding, decoding, round-trips - isFloat16Lossless, isFloat32Lossless, isFloat64Lossless checks - Cross-platform consistency tests (JS-specific bug fixes) - Edge cases: byte array lengths, big-endian order 105 new tests, all passing on VM, JS (dart2js), and WASM (dart2wasm). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: add ieee754 validation tests for float_utils Add ieee754 package as dev dependency and use it to validate that the custom float_utils implementation produces identical results to ieee754 on non-JS platforms. Validation is performed inline within each existing test using helper functions that compare results against ieee754 (skipped on JS where ieee754 has known issues). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #85 +/- ##
==========================================
- Coverage 70.97% 66.50% -4.48%
==========================================
Files 23 26 +3
Lines 1237 1827 +590
==========================================
+ Hits 878 1215 +337
- Misses 359 612 +253 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Owner
Author
|
@AlexDochioiu as you can see code cov is has gone down a bit, personally I'm OK with this as I am with the rest of the PR. If you are happy I'll merge this or feel free to look at the code coverage if you wish. |
AlexDochioiu
approved these changes
Jan 24, 2026
Contributor
|
I think that's fine @shamblett . Maybe we can leave a new issue open to increase test coverage. |
Owner
Author
|
Done, #86 raised to investigate and correct test coverage. |
Owner
Author
|
Package re published at version 6.5.0. |
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.
No description provided.