fix(torii-core): client negative i128 conversion - #2840
Conversation
WalkthroughOhayo, sensei! The pull request introduces a modification in the Changes
Suggested reviewers
Possibly related PRs
The change is straightforward but demonstrates careful attention to type conversion nuances in Rust. Sensei would be proud of such meticulous code refinement! 🦀✨ 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/torii/core/src/model.rs (1)
273-276: Consider adding test coverage for negative i128 values, sensei!While the implementation is correct, it would be beneficial to add test cases that explicitly verify the conversion of negative i128 values from their hexadecimal representation.
Would you like me to help create test cases that cover:
- Maximum negative value (-2^127)
- Maximum positive value (2^127 - 1)
- Common negative values
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/torii/core/src/model.rs(1 hunks)
🔇 Additional comments (1)
crates/torii/core/src/model.rs (1)
273-276: Ohayo sensei! The i128 conversion implementation looks solid.
The two-step conversion approach (hex → u128 → i128) aligns well with how other large integers are handled in the codebase. This implementation ensures proper handling of the full range of i128 values, including negative numbers.
Let's verify the handling of edge cases:
✅ Verification successful
Ohayo sensei! The i128 conversion implementation is verified as correct and consistent.
The codebase analysis reveals:
- The conversion from hex string to i128 via u128 is consistent with how other large integers are handled
- Tests in
crates/dojo/types/src/primitive_conversion.rsverify i128 conversion with edge cases including negative values (e.g. -170141183460469231731687303715884105728) - The implementation is used consistently across the codebase for handling i128 values in Primitive types
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for test cases that exercise i128 conversion edge cases
# Expected: Tests covering negative values and boundary conditions
# Search for i128-related test cases
rg -A 5 "i128|I128" "tests/"
# Search for hex string constants that might be used in tests
rg -A 2 "0x[0-9a-fA-F]+" "tests/"
Length of output: 156
Script:
#!/bin/bash
# Let's search in the entire codebase for test files and i128 related tests
fd -e rs | xargs rg -l "test" | xargs rg -A 5 "i128|I128"
# Search for hex string test cases in all Rust files
fd -e rs | xargs rg -A 2 "0x[0-9a-fA-F]+"
# Look for primitive type conversion tests
fd -e rs | xargs rg -A 5 "Primitive::(I128|U128)"
Length of output: 82241
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2840 +/- ##
=======================================
Coverage 56.27% 56.28%
=======================================
Files 439 439
Lines 56276 56274 -2
=======================================
+ Hits 31671 31673 +2
+ Misses 24605 24601 -4 ☔ View full report in Codecov by Sentry. |
2 participants
Summary by CodeRabbit
I128primitive type, enhancing data population from SQLite rows.