feat: add binary-safe compression and decompression APIs and implemen… - #1
feat: add binary-safe compression and decompression APIs and implemen…#1henningpokriefke wants to merge 7 commits into
Conversation
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds binary-safe ArrayBuffer compress/decompress APIs in C++ and JS, introduces codec interfaces and two adapters, exposes a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant JSIndex as index.tsx
participant Nitro as NitroModule
participant NativeCPP as HybridZstd (C++)
Client->>JSIndex: compressBytes(ArrayBuffer)
JSIndex->>Nitro: call compressBytes(ArrayBuffer)
Nitro->>NativeCPP: invoke HybridZstd::compressBytes
NativeCPP->>NativeCPP: allocate output buffer and compress
NativeCPP-->>Nitro: return compressed ArrayBuffer
Nitro-->>JSIndex: compressed data
JSIndex-->>Client: return ArrayBuffer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In @src/__tests__/backward-compat.test.ts:
- Around line 10-48: Duplicate mock implementation found for
NitroModules.createHybridObject across tests; extract the mock into a shared
helper and replace inline mocks with calls to it. Create a helper that exports
createMockHybridObject() (returning the
compress/decompress/compressBytes/decompressBytes implementations) and
mockNitroModules() (returning { NitroModules: { createHybridObject: () =>
createMockHybridObject() } }), then update test files to import mockNitroModules
and use jest.mock('react-native-nitro-modules', () => mockNitroModules())
instead of duplicating the implementation.
In @src/__tests__/binary.test.ts:
- Around line 1-43: The tests duplicate the same mock implementation for the
native module (functions compress, decompress, compressBytes, decompressBytes)
across multiple test files; extract that mock into a shared test utility or
fixture (e.g., a module export like createNitroMock or setupNitroMocks) and
update binary.test.ts, fuzz.test.ts, and backward-compat.test.ts to import and
use the shared mock setup so the header-prepending/prep removal behavior is
centralized and maintained in one place.
In @src/__tests__/codecs.test.ts:
- Around line 193-199: Add a test that verifies ZstdBinaryCompression correctly
round-trips an empty Uint8Array: create an empty Uint8Array, call the adapter's
compress and then decompress (use the ZstdBinaryCompression or
ZstdBinaryCompressionAdapter instance used in the suite, e.g., variable named
`binaryCodec`/`codec`), and assert the decompressed result is an empty
Uint8Array (or has length 0). Place this alongside the existing binary-safety
tests so empty-byte behavior is covered consistently with
ZstdTextCompressionAdapter.
In @src/__tests__/fuzz.test.ts:
- Around line 56-61: The test inconsistently uses the TypeScript assertion "as
ArrayBuffer" on generateRandomBytes().buffer; make it consistent by removing the
redundant "as ArrayBuffer" casts where the buffer is directly owned (e.g., in
the calls to compressBytes in the tests using generateRandomBytes), so change
calls using compressBytes(input.buffer as ArrayBuffer, ...) to
compressBytes(input.buffer, ...), and apply the same removal for any other
places referencing input.buffer (e.g., decompressBytes inputs) to keep
generateRandomBytes, input.buffer, compressBytes and decompressBytes usage
uniform and more readable.
- Around line 41-48: The test uses non-deterministic Math.random() in
generateRandomBytes which causes flaky tests; replace it with a seeded PRNG so
runs are reproducible (e.g., add an optional seed parameter to
generateRandomBytes and use a simple LCG or a test-seeding library like
seedrandom to generate bytes deterministically), update any tests that call
generateRandomBytes to pass a fixed seed, and ensure the PRNG state is local to
generateRandomBytes to avoid global side effects.
In @src/__tests__/index.test.tsx:
- Around line 23-52: Tests repeatedly perform dynamic imports of the same
module; consolidate by importing the module once in a shared setup (e.g., a
beforeAll) and reuse the exported symbols (compress, decompress, compressBytes,
decompressBytes, ZstdBinaryCompression, ZstdTextCompressionAdapter) in each it
block to avoid redundant imports and improve test efficiency.
In @src/codecs/ZstdTextCompressionAdapter.ts:
- Around line 18-23: The compress method in ZstdTextCompressionAdapter currently
decodes bytes with TextDecoder without the fatal option, which silently replaces
invalid UTF-8; update the decode to use new TextDecoder('utf-8', { fatal: true
}) and wrap the decode call in a try/catch so invalid UTF-8 throws a clear error
(include context like "ZstdTextCompressionAdapter.compress" and the provided
defaultLevel) before proceeding to call compress(text, level ??
this.defaultLevel) and returning the Uint8Array.
In @src/index.tsx:
- Around line 6-21: The parameter name is inconsistent between compress
(compressLevel) and compressBytes (compressionLevel); to fix, rename the
compressBytes parameter to compressLevel and update its internal call to
ZstdHybridObject.compressBytes(data, compressLevel) so the exported API matches
the existing naming; if you must preserve backward compatibility, instead add a
thin wrapper overload named compressBytes(data: ArrayBuffer, compressLevel:
number = 3) that forwards to the current implementation (or keep
compressionLevel and add an aliased function/composed wrapper named
compressBytesWithCompressLevel) so both parameter names work.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (13)
nitrogen/generated/android/kotlin/com/margelo/nitro/zstd/reactnativezstdOnLoad.ktis excluded by!**/generated/**nitrogen/generated/android/reactnativezstd+autolinking.cmakeis excluded by!**/generated/**nitrogen/generated/android/reactnativezstd+autolinking.gradleis excluded by!**/generated/**nitrogen/generated/android/reactnativezstdOnLoad.cppis excluded by!**/generated/**nitrogen/generated/android/reactnativezstdOnLoad.hppis excluded by!**/generated/**nitrogen/generated/ios/Zstd+autolinking.rbis excluded by!**/generated/**nitrogen/generated/ios/Zstd-Swift-Cxx-Bridge.cppis excluded by!**/generated/**nitrogen/generated/ios/Zstd-Swift-Cxx-Bridge.hppis excluded by!**/generated/**nitrogen/generated/ios/Zstd-Swift-Cxx-Umbrella.hppis excluded by!**/generated/**nitrogen/generated/ios/ZstdAutolinking.mmis excluded by!**/generated/**nitrogen/generated/ios/ZstdAutolinking.swiftis excluded by!**/generated/**nitrogen/generated/shared/c++/HybridZstdSpec.cppis excluded by!**/generated/**nitrogen/generated/shared/c++/HybridZstdSpec.hppis excluded by!**/generated/**
📒 Files selected for processing (14)
cpp/HybridZstd.cppcpp/HybridZstd.hpppackage.jsonsrc/Zstd.nitro.tssrc/__tests__/backward-compat.test.tssrc/__tests__/binary.test.tssrc/__tests__/codecs.test.tssrc/__tests__/fuzz.test.tssrc/__tests__/index.test.tsxsrc/codecs/ZstdBinaryCompression.tssrc/codecs/ZstdTextCompressionAdapter.tssrc/codecs/index.tssrc/codecs/types.tssrc/index.tsx
🧰 Additional context used
🧬 Code graph analysis (10)
src/codecs/types.ts (2)
src/codecs/index.ts (2)
BlobCodecV1(2-2)CompressionV1(2-2)src/index.tsx (2)
BlobCodecV1(29-29)CompressionV1(29-29)
src/Zstd.nitro.ts (2)
cpp/HybridZstd.hpp (4)
data(12-12)data(13-13)data(16-16)data(17-17)nitrogen/generated/shared/c++/HybridZstdSpec.hpp (4)
data(52-52)data(53-53)data(54-54)data(55-55)
src/__tests__/binary.test.ts (3)
cpp/HybridZstd.hpp (4)
data(12-12)data(13-13)data(16-16)data(17-17)cpp/HybridZstd.cpp (4)
compressBytes(54-81)compressBytes(54-56)decompressBytes(84-109)decompressBytes(84-85)src/index.tsx (2)
compressBytes(16-21)decompressBytes(23-25)
src/codecs/ZstdTextCompressionAdapter.ts (3)
src/index.tsx (3)
CompressionV1(29-29)compress(7-9)decompress(11-13)src/codecs/types.ts (1)
CompressionV1(22-38)src/codecs/ZstdBinaryCompression.ts (2)
compress(18-28)decompress(30-40)
src/__tests__/codecs.test.ts (3)
src/codecs/types.ts (1)
CompressionV1(22-38)src/codecs/ZstdBinaryCompression.ts (1)
ZstdBinaryCompression(11-41)src/codecs/ZstdTextCompressionAdapter.ts (1)
ZstdTextCompressionAdapter(11-34)
src/__tests__/fuzz.test.ts (3)
cpp/HybridZstd.hpp (4)
data(12-12)data(13-13)data(16-16)data(17-17)cpp/HybridZstd.cpp (5)
result(47-47)compressBytes(54-81)compressBytes(54-56)decompressBytes(84-109)decompressBytes(84-85)src/index.tsx (2)
compressBytes(16-21)decompressBytes(23-25)
src/__tests__/index.test.tsx (3)
src/codecs/ZstdBinaryCompression.ts (3)
compress(18-28)decompress(30-40)ZstdBinaryCompression(11-41)src/codecs/ZstdTextCompressionAdapter.ts (3)
compress(18-23)decompress(25-33)ZstdTextCompressionAdapter(11-34)src/index.tsx (6)
compress(7-9)decompress(11-13)compressBytes(16-21)decompressBytes(23-25)ZstdBinaryCompression(28-28)ZstdTextCompressionAdapter(28-28)
src/__tests__/backward-compat.test.ts (1)
src/index.tsx (4)
compress(7-9)decompress(11-13)compressBytes(16-21)decompressBytes(23-25)
src/codecs/ZstdBinaryCompression.ts (3)
src/codecs/index.ts (2)
ZstdBinaryCompression(6-6)CompressionV1(2-2)src/index.tsx (4)
ZstdBinaryCompression(28-28)CompressionV1(29-29)compressBytes(16-21)decompressBytes(23-25)src/codecs/types.ts (1)
CompressionV1(22-38)
cpp/HybridZstd.hpp (1)
nitrogen/generated/shared/c++/HybridZstdSpec.hpp (4)
data(52-52)data(53-53)data(54-54)data(55-55)
🪛 Clang (14.0.6)
cpp/HybridZstd.cpp
[warning] 54-54: use a trailing return type for this function
(modernize-use-trailing-return-type)
[warning] 55-55: 2 adjacent parameters of 'compressBytes' of convertible types are easily swapped by mistake
(bugprone-easily-swappable-parameters)
[note] 55-55: the first parameter in the range is 'data'
(clang)
[note] 56-56: the last parameter in the range is 'compressionLevel'
(clang)
[note] 56-56: 'const int &' and 'double' parameters accept and bind the same kind of values
(clang)
[note] 56-56: 'const int &' and 'double' may be implicitly converted: 'const int &' (as 'int') -> 'double', 'double' -> 'const int &' (as 'int')
(clang)
[warning] 60-60: variable 'compressedData' is not initialized
(cppcoreguidelines-init-variables)
[warning] 84-84: use a trailing return type for this function
(modernize-use-trailing-return-type)
[warning] 89-89: variable 'decompressedData' is not initialized
(cppcoreguidelines-init-variables)
cpp/HybridZstd.hpp
[warning] 12-12: use a trailing return type for this function
(modernize-use-trailing-return-type)
[warning] 13-13: use a trailing return type for this function
(modernize-use-trailing-return-type)
[warning] 16-16: use a trailing return type for this function
(modernize-use-trailing-return-type)
[warning] 17-17: use a trailing return type for this function
(modernize-use-trailing-return-type)
🔇 Additional comments (20)
package.json (1)
13-17: LGTM!The new
./codecsexport entry follows the established pattern and correctly maps source, types, and module paths for the new codec adapters.cpp/HybridZstd.hpp (1)
15-17: LGTM!The binary-safe API declarations correctly match the generated
HybridZstdSpec.hppsignatures and maintain consistency with the existing string-based API pattern.src/__tests__/index.test.tsx (1)
11-21: LGTM!The mock correctly provides stub implementations for all four native methods, enabling unit tests to verify export presence without requiring the native module.
src/Zstd.nitro.ts (1)
8-10: LGTM!The binary-safe API additions are well-designed with clear naming (
compressBytes/decompressBytes) and appropriateArrayBuffertypes that mirror the C++ implementation.cpp/HybridZstd.cpp (2)
53-81: LGTM!The
compressBytesimplementation correctly mirrors the string-basedcompressfunction with proper memory management. The null checks and cleanup on allocation failure are appropriate.
83-109: No bug found in the TypeScript adapter—both compress and decompress use identical, correct buffer slicing.The C++ implementation is correct. However, the claimed TypeScript bug does not exist. Both the
compress(lines 21–24) anddecompress(lines 33–36) methods inZstdBinaryCompression.tsuse the same pattern:bytes.buffer.slice(bytes.byteOffset, bytes.byteOffset + bytes.byteLength), which correctly handles typed array views with non-zero offsets. There is no inconsistency.Likely an incorrect or invalid review comment.
src/codecs/index.ts (1)
1-6: LGTM!Clean barrel export with proper separation of type exports and value exports. The organization is clear and follows TypeScript best practices.
src/codecs/types.ts (1)
1-38: LGTM!Well-designed interfaces with clear separation of concerns:
BlobCodecV1for serialization (objects ↔ bytes)CompressionV1for compression (bytes ↔ compressed bytes)The versioned naming convention (V1) provides a good foundation for future API evolution without breaking changes. Documentation is thorough with appropriate
@throwsannotations.src/codecs/ZstdTextCompressionAdapter.ts (1)
25-33: LGTM!The decompress method correctly handles
Uint8Arrayviews by properly slicing the underlying buffer withbyteOffsetandbyteLength. This ensures correct behavior when the input is a view into a larger buffer.src/__tests__/codecs.test.ts (4)
11-44: LGTM!The mock implementation is well-designed, simulating ZSTD compression by prepending a header (
"ZSTD"magic bytes) and preserving the payload. This allows round-trip testing without requiring the actual native module.
73-97: Excellent binary safety coverage.The tests properly verify that null bytes, high-value bytes (0xff), and the full byte range (0-255) round-trip correctly. This is critical validation for the binary-safe API claims.
123-137: LGTM!Good coverage of the edge case where
Uint8Arrayis a view into a larger buffer with a non-zero offset. This validates that the implementations correctly usebyteOffsetandbyteLengthrather than assuming the view starts at the beginning of the underlying buffer.
220-246: LGTM!Good adapter parity tests ensuring both implementations conform to the same interface and produce equivalent results for text data. This validates the interoperability story.
src/index.tsx (1)
27-29: LGTM!Clean re-exports of the codec module, making adapters and types accessible from the main entry point. The separation of value exports from type exports is correct.
src/__tests__/binary.test.ts (1)
45-138: Comprehensive test coverage for binary compression API.The test suite effectively covers:
- Round-trip integrity for various byte patterns including null bytes (0x00) and high bytes (0xFF)
- Full byte-range coverage (0x00-0xFF) ensuring all byte values are preserved
- API contract validation (return types)
- Compression level boundaries (1 and 19)
This provides good confidence that the binary API correctly handles arbitrary byte sequences.
src/__tests__/fuzz.test.ts (1)
64-122: Good coverage of edge cases and specific patterns.The tests effectively cover:
- Multiple iterations with varying random sizes
- Power-of-2 boundary sizes (important for buffer allocation)
- Repeating patterns (exercises compression dictionary)
- Incrementing sequences
- Mixed null/random bytes (validates binary safety)
src/codecs/ZstdBinaryCompression.ts (2)
18-28: Correct handling of Uint8Array views.The implementation correctly uses
bytes.buffer.slice(bytes.byteOffset, bytes.byteOffset + bytes.byteLength)to handle cases where the inputUint8Arrayis a view into a largerArrayBuffer. This ensures only the relevant portion is processed, which is critical for binary safety.The
as ArrayBuffercast on line 24 is safe becauseArrayBuffer.prototype.slicealways returns anArrayBuffer.
30-40: Consistent implementation matching the compress method.The decompress method follows the same pattern for handling Uint8Array views. The absence of try-catch is appropriate here since the
CompressionV1interface documents thatdecompressthrows on failure, delegating error handling to the caller.src/__tests__/backward-compat.test.ts (2)
50-102: Thorough backward compatibility coverage for string API.The test suite properly validates that the existing string-based API remains unchanged:
- Input/output types preserved
- Round-trip integrity for various text inputs including UTF-8
- Default compression level behavior
This ensures the new binary API addition doesn't break existing consumers.
124-154: Good interoperability tests with appropriate mock caveats.The interoperability tests verify practical use cases:
- Both APIs produce ArrayBuffer outputs
- Binary-compressed text can be decoded back to string after decompression
The comment on lines 136-137 correctly notes this equality test relies on mock behavior. In production, the native string API may handle encoding differently, so this byte-equality assertion is appropriate only for verifying mock consistency.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/__tests__/fuzz.test.ts:
- Around line 121-130: The test 'handles mixed null and random bytes' uses
Math.random() twice causing non-deterministic behavior; replace both
Math.random() calls with a seeded PRNG so the generated Uint8Array is
reproducible across runs (keep the same distribution: ~50% null vs random byte).
Update the loop in the test to initialize a deterministic RNG (or use an
imported seeded utility) and call it for both the null-vs-random decision and
the byte value generation, leaving calls to compressBytes and decompressBytes
unchanged and asserting equality as before.
- Around line 74-82: The test uses Math.random() to pick iteration sizes, making
the fuzzing non-deterministic; replace Math.random() with the file's seeded PRNG
function (the seeded generator defined earlier in this test file) so sizes are
reproducible across runs — e.g. call the existing seeded generator instead of
Math.random() when computing size in the loop that creates input via
generateRandomBytes, keeping the rest of the test that calls compressBytes and
decompressBytes unchanged.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/__tests__/fuzz.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/__tests__/fuzz.test.ts (3)
cpp/HybridZstd.hpp (4)
data(12-12)data(13-13)data(16-16)data(17-17)cpp/HybridZstd.cpp (4)
compressBytes(54-81)compressBytes(54-56)decompressBytes(84-109)decompressBytes(84-85)src/index.tsx (2)
compressBytes(16-21)decompressBytes(23-25)
🔇 Additional comments (4)
src/__tests__/fuzz.test.ts (4)
1-9: LGTM!Clear documentation header and correct imports for the binary compression APIs.
10-39: LGTM!The mock implementation correctly simulates binary compression behavior with a deterministic header-based approach, enabling reliable unit tests without the native module.
41-57: LGTM!The seeded PRNG uses well-known LCG constants and
generateRandomBytescorrectly produces reproducible random data for testing.
59-71: LGTM!Good parameterized test coverage across multiple data sizes with reproducible random data.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…ura/react-native-zstd into adaptura/binary-zstd-codec
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/__tests__/fuzz.test.ts:
- Around line 124-133: The test "handles mixed null and random bytes" is
non-deterministic because it uses Math.random() (twice per loop); replace
Math.random() with a seeded PRNG used throughout the test so the byte pattern is
reproducible; update the test to create a seeded RNG (e.g., seedrandom or a
simple LCG) at the top of the test, call that rng() instead of Math.random()
when deciding null vs random and when generating the random byte, and then run
compressBytes/decompressBytes as before to assert equality.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/__tests__/codecs.test.tssrc/__tests__/fuzz.test.tssrc/index.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
src/__tests__/fuzz.test.ts (3)
cpp/HybridZstd.hpp (4)
data(12-12)data(13-13)data(16-16)data(17-17)cpp/HybridZstd.cpp (4)
compressBytes(54-81)compressBytes(54-56)decompressBytes(84-109)decompressBytes(84-85)src/index.tsx (2)
compressBytes(16-21)decompressBytes(23-25)
src/index.tsx (5)
src/codecs/ZstdBinaryCompression.ts (2)
compress(18-28)decompress(30-40)src/codecs/ZstdTextCompressionAdapter.ts (2)
compress(18-23)decompress(25-33)cpp/react-native-zstd.cpp (4)
compress(10-35)compress(10-13)decompress(38-80)decompress(38-40)cpp/HybridZstd.hpp (4)
data(12-12)data(13-13)data(16-16)data(17-17)nitrogen/generated/shared/c++/HybridZstdSpec.hpp (4)
data(52-52)data(53-53)data(54-54)data(55-55)
src/__tests__/codecs.test.ts (3)
src/codecs/types.ts (1)
CompressionV1(22-38)src/codecs/ZstdBinaryCompression.ts (1)
ZstdBinaryCompression(11-41)src/codecs/ZstdTextCompressionAdapter.ts (1)
ZstdTextCompressionAdapter(11-34)
🔇 Additional comments (19)
src/index.tsx (3)
6-13: LGTM!The string-based API is clearly marked for backward compatibility, and the implementation correctly delegates to the hybrid object methods.
15-25: LGTM!The binary-safe API is well-structured and mirrors the string-based API pattern. The signatures correctly align with the C++ HybridZstdSpec (
compressBytes(ArrayBuffer, double)anddecompressBytes(ArrayBuffer)). The default compression level of 3 is consistent across both APIs.
27-29: LGTM!Clean re-exports of codec adapters and types, providing a unified public API surface from the main entry point.
src/__tests__/fuzz.test.ts (6)
1-9: LGTM!Clear documentation header explaining the purpose of the fuzz tests.
10-39: LGTM!The mock correctly simulates binary compression behavior with a 4-byte header (
0x5a, 0x53, 0x54, 0x44= "ZSTD"), providing a consistent round-trip mechanism for testing without the native module.
41-60: LGTM!The seeded PRNG implementation using a linear congruential generator provides reproducible random data generation, which is essential for reliable fuzz testing.
62-74: LGTM!Well-structured parameterized tests covering a good range of input sizes from tiny to large.
87-99: LGTM!Good coverage of edge-case sizes around powers of two and boundary values.
101-122: LGTM!Good coverage of deterministic byte patterns that could trigger compression edge cases.
src/__tests__/codecs.test.ts (10)
1-9: LGTM!Clear documentation and proper imports of both codec adapters and the interface type.
11-44: LGTM!The mock is consistent with
fuzz.test.tsand correctly simulates both string-based and binary compression paths using the same header convention.
46-71: LGTM!Interface compliance tests properly verify that the codec exposes the expected methods and returns correct types.
73-105: LGTM!Excellent coverage of binary safety edge cases including null bytes, 0xff bytes, all byte values (0-255), and empty arrays. These tests validate the core binary-safe behavior that distinguishes the new API from the string-based API.
107-129: LGTM!Good coverage of compression level handling through constructor, per-call parameter, and default behavior.
131-146: LGTM!This is an important test case. It validates that
ZstdBinaryCompressioncorrectly handlesUint8Arrayviews with non-zero offsets, which aligns with the implementation insrc/codecs/ZstdBinaryCompression.tsthat usesbytes.buffer.slice(bytes.byteOffset, bytes.byteOffset + bytes.byteLength).
148-173: LGTM!Interface compliance tests for the text adapter mirror the binary adapter tests appropriately.
175-208: LGTM!Comprehensive text handling tests covering UTF-8, JSON, special characters (including multi-byte Unicode and emoji), and empty strings.
210-226: LGTM!Compression level tests for the text adapter are consistent with the binary adapter tests.
228-254: LGTM!The adapter comparison tests ensure both implementations conform to the same
CompressionV1interface and produce equivalent results for text data. This validates interoperability between the two adapters.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/__tests__/fuzz.test.ts:
- Around line 32-36: The mock decompressBytes assumes at least 4 bytes and will
throw for empty or short ArrayBuffers; add a guard in the decompressBytes
implementation to check the input length (via new Uint8Array(data).length) and
handle cases where length < 4 by returning an empty ArrayBuffer (or the
appropriate empty/identity value your code expects) instead of slicing, and add
a unit test in fuzz.test.ts that calls decompressBytes with an empty ArrayBuffer
to assert the expected behavior.
- Around line 102-136: Add two additional test cases to the existing "specific
byte patterns" suite to cover all-zeros and all-0xFF buffers: create a 1000-byte
Uint8Array filled with 0x00 and another filled with 0xFF, compress them with
compressBytes(..., 3), decompress with decompressBytes(...), and assert the
decompressed output equals the original buffer; reuse the existing style and
helper functions (compressBytes, decompressBytes, createSeededRandom) and place
the new it(...) blocks alongside the repeating/incrementing/mixed tests.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/__tests__/fuzz.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/__tests__/fuzz.test.ts (2)
cpp/HybridZstd.cpp (4)
compressBytes(54-81)compressBytes(54-56)decompressBytes(84-109)decompressBytes(84-85)src/index.tsx (2)
compressBytes(16-21)decompressBytes(23-25)
🔇 Additional comments (2)
src/__tests__/fuzz.test.ts (2)
62-74: LGTM!Good coverage of size categories from tiny to large. The parameterized test structure is clean and maintainable.
76-100: LGTM!Good use of seeded randomness for reproducibility. Edge-case sizes around powers of two are valuable for catching buffer boundary issues.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/__tests__/fuzz.test.ts:
- Around line 45-51: The PRNG produced by createSeededRandom can return exactly
1.0 when seed equals 0x7fffffff, causing downstream Math.floor(random() * 256)
to yield 256; to fix it, change the divisor in the returned function from
0x7fffffff to 0x80000000 so the generated value is in [0,1) (i.e., update the
expression that returns seed / 0x7fffffff to use seed / 0x80000000), leaving the
LCG update and masking logic in createSeededRandom unchanged.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/__tests__/fuzz.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/__tests__/fuzz.test.ts (2)
cpp/HybridZstd.cpp (4)
compressBytes(54-81)compressBytes(54-56)decompressBytes(84-109)decompressBytes(84-85)src/index.tsx (2)
compressBytes(16-21)decompressBytes(23-25)
🔇 Additional comments (6)
src/__tests__/fuzz.test.ts (6)
1-9: LGTM!Clear documentation and appropriate imports for the fuzz test suite.
10-43: Well-structured mock with proper edge case handling.The mock correctly simulates round-trip behavior with header-based approach. The guard for short buffers on line 36-38 properly handles edge cases, returning an empty ArrayBuffer instead of throwing on malformed input.
53-64: Helper function looks correct.The
generateRandomByteshelper properly creates deterministic test data. The potential byte overflow issue is addressed in the PRNG comment above.
66-78: Good coverage of size ranges.The parameterized test properly validates round-trip integrity across different data sizes. The
as ArrayBuffercast is safe here sincegenerateRandomBytescreates freshUint8Arrayinstances with matching buffer sizes.
80-104: Excellent edge-case coverage.The edge-case size list thoughtfully includes power-of-two boundaries and their neighbors, which commonly trigger buffer-handling issues. Using the iteration index as seed ensures reproducible yet unique test data.
106-162: Comprehensive byte pattern coverage.The test suite covers critical compression edge cases:
- Repeating patterns that may trigger compression dictionary behavior
- Incrementing sequences that test sequential byte handling
- Mixed null bytes that verify binary-safety (no null termination issues)
- Uniform buffers (all-zeros, all-0xFF) that stress run-length encoding
- Empty buffer handling
The empty
ArrayBuffertest at lines 155-161 is particularly valuable for validating the guard in the mock'sdecompressBytes.
| // Simple seeded PRNG for reproducible tests | ||
| const createSeededRandom = (seed: number) => { | ||
| return () => { | ||
| seed = (seed * 1103515245 + 12345) & 0x7fffffff; | ||
| return seed / 0x7fffffff; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Potential off-by-one: random() can return exactly 1.0.
When seed equals 0x7fffffff after the bitwise AND, dividing by 0x7fffffff returns exactly 1.0. This causes Math.floor(random() * 256) to return 256, which is outside the valid byte range [0, 255].
Proposed fix
const createSeededRandom = (seed: number) => {
return () => {
seed = (seed * 1103515245 + 12345) & 0x7fffffff;
- return seed / 0x7fffffff;
+ return seed / 0x80000000;
};
};Dividing by 0x80000000 ensures the result is strictly in [0, 1).
🤖 Prompt for AI Agents
In @src/__tests__/fuzz.test.ts around lines 45 - 51, The PRNG produced by
createSeededRandom can return exactly 1.0 when seed equals 0x7fffffff, causing
downstream Math.floor(random() * 256) to yield 256; to fix it, change the
divisor in the returned function from 0x7fffffff to 0x80000000 so the generated
value is in [0,1) (i.e., update the expression that returns seed / 0x7fffffff to
use seed / 0x80000000), leaving the LCG update and masking logic in
createSeededRandom unchanged.
1 participant
…t codec adapters