ob int #2139
Merged
ob int #2139
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
Contributor
WalkthroughUpdated two submodule pointers and replaced numerous Changes
Sequence Diagram(s)(omitted) — changes are formatting, tests, and submodule pointer updates only; no control-flow or interaction changes to visualize. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
lib/rain.orderbook.interface(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1913
File: Cargo.toml:55-55
Timestamp: 2025-06-06T16:38:04.182Z
Learning: In rain.orderbook repository, during PR chains involving dependency updates, wasm-bindgen-utils may temporarily point to git commits that don't contain the full required functionality, with the understanding that subsequent PRs in the chain will update it to the correct commit.
📚 Learning: 2025-07-16T10:40:05.717Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Applied to files:
lib/rain.orderbook.interface
📚 Learning: 2025-07-04T09:02:57.301Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1956
File: crates/common/src/fuzz/mod.rs:64-64
Timestamp: 2025-07-04T09:02:57.301Z
Learning: In rainlanguage/rain.orderbook, user findolor prefers to limit type consistency changes to only the parts directly related to the current work scope. For example, when updating chain_id fields from u64 to u32 in fuzz-related code, unrelated files like tauri-app wallet commands can remain as u64 if they serve different purposes and aren't part of the current changes.
Applied to files:
lib/rain.orderbook.interface
📚 Learning: 2025-06-18T12:54:47.807Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1938
File: crates/js_api/src/raindex/orders.rs:1-11
Timestamp: 2025-06-18T12:54:47.807Z
Learning: In the rain.orderbook repository, findolor prefers to keep unused imports in place when they will be used in upcoming follow-up PRs, rather than removing and re-adding them during incremental development.
Applied to files:
lib/rain.orderbook.interface
| @@ -1 +1 @@ | |||
| Subproject commit 21f3e97c033e1533641be27f7b21e9f828b42ce8 | |||
| Subproject commit 71612d2bb8fc2e10047f557e9c8c30ab103b35b0 | |||
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Submodule bump: verify 71612d2 provenance and surface the delta
Please confirm this SHA is from the intended upstream branch/tag and that the delta is expected. Add a brief Motivation/Solution in the PR body linking to the upstream changes or tag for traceability. If this is an interim pointer as part of a PR chain (per prior repo practice), call that out explicitly to avoid confusion.
Run this to inspect old vs new submodule SHAs, URL, and the changelog between them:
Follow-ups:
- If the range includes breaking changes, note any required downstream updates in this repo.
- Consider referencing a signed tag/release in the PR description for reproducibility, even while pinning to a commit.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
SUBPATH="lib/rain.orderbook.interface"
# Ensure we have the base to compare against
git fetch origin main --quiet
BASE_COMMIT="$(git merge-base HEAD origin/main)"
OLD_SHA="$(git ls-tree "$BASE_COMMIT" "$SUBPATH" | awk '{print $3}')"
NEW_SHA="$(git ls-tree HEAD "$SUBPATH" | awk '{print $3}')"
echo "Submodule: $SUBPATH"
git config -f .gitmodules --get "submodule.$SUBPATH.url" || true
echo "OLD: $OLD_SHA (from origin/main)"
echo "NEW: $NEW_SHA (from HEAD)"
# Initialize and fetch the submodule to view logs
git submodule update --init --depth 1 "$SUBPATH"
git -C "$SUBPATH" fetch --depth 500 origin
echo
echo "Log (OLD..NEW):"
git -C "$SUBPATH" log --oneline --decorate --graph "$OLD_SHA".."$NEW_SHA" -n 100 || true
echo
echo "Potential breaking markers:"
git -C "$SUBPATH" log --oneline "$OLD_SHA".."$NEW_SHA" | rg -nP '(?i)breaking|feat|fix' || trueLength of output: 765
Submodule bump verified—commit 71612d2 from upstream main
No breaking-change markers found between 21f3e97 and 71612d2. Please add a brief Motivation/Solution to the PR description linking to upstream PR #40 (2025-09-09-int) for traceability and call out if this is an interim pointer.
🤖 Prompt for AI Agents
In lib/rain.orderbook.interface around lines 1 to 1, the PR lacks a brief
Motivation/Solution note and traceability to the upstream change; update the PR
description to include a short "Motivation" and "Solution" section that
references upstream PR #40 (commit 71612d2) with the date (2025-09-09-int),
state whether this is an interim pointer or a final change, and link or cite the
upstream commit/PR for traceability.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
lib/rain.interpreter(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1913
File: Cargo.toml:55-55
Timestamp: 2025-06-06T16:38:04.182Z
Learning: In rain.orderbook repository, during PR chains involving dependency updates, wasm-bindgen-utils may temporarily point to git commits that don't contain the full required functionality, with the understanding that subsequent PRs in the chain will update it to the correct commit.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1938
File: crates/js_api/src/raindex/orders.rs:1-11
Timestamp: 2025-06-18T12:54:47.807Z
Learning: In the rain.orderbook repository, findolor prefers to keep unused imports in place when they will be used in upcoming follow-up PRs, rather than removing and re-adding them during incremental development.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T23:09:46.344Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must reassess the PR size after each new commit to ensure the report includes the latest changes, and calculate the total size up to the very latest commit.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: git-clean
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: test
- GitHub Check: Deploy-Preview-Push
- GitHub Check: Deploy-Docs-Preview
🔇 Additional comments (1)
lib/rain.interpreter (1)
1-1: Ensure CI workflows checkout submodules recursively after bumping rain.interpreter to a18536e48177b46e6611f8180729ba1519504a11
- Verified new commit is reachable upstream and
git submodule update --init --recursivesucceeds- Changelog (474fc037…→a18536e48177):
• 97c01965 fix test
• 4d1fd239 pionters
• 71bbbffc bump int
• e5a3f58e Update rain.metadata- No tags contain the new commit
- CI workflows currently lack explicit submodule recursion—add under your
actions/checkoutstep in .github/workflows:- uses: actions/checkout@v3 with: submodules: recursive[mandatory_fixes_required]
| @@ -1 +1 @@ | |||
| Subproject commit 474fc0374534388993d0b712673a476636eb5488 | |||
| Subproject commit a18536e48177b46e6611f8180729ba1519504a11 | |||
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
PR hygiene: fill Motivation/Solution and checkboxes for this submodule-only change.
Add 1–2 lines on why this commit is needed (bugfix, perf, API alignment) and any downstream risk. Link the upstream interpreter commit/PR to speed review. Not blocking, but it helps traceability.
I can draft the PR description and a release-note entry if you share the intent of this bump.
🤖 Prompt for AI Agents
In lib/rain.interpreter around lines 1 to 1, the commit/PR lacks a short
Motivation/Solution and checkbox verification for this submodule-only bump;
update the PR description (or commit message) to add 1–2 lines stating why the
bump is required (bugfix, perf, API alignment), note any downstream risk, and
include a link to the upstream interpreter commit/PR for traceability; also
complete the requested checkboxes (e.g., tests, changelog, release notes) or
mark them as not applicable so the PR passes hygiene checks.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
crates/common/src/raindex_client/vaults.rs(3 hunks)crates/common/src/types/token_vault_flattened.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
crates/**/{src,tests,benches,examples}/**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
crates/**/{src,tests,benches,examples}/**/*.rs: Rust crates: lint withcargo clippy --workspace --all-targets --all-features -D warnings
Rust crates: ensurecargo test --workspacepasses
Rust crates: build withcargo buildduring development/readiness checks
Files:
crates/common/src/raindex_client/vaults.rscrates/common/src/types/token_vault_flattened.rs
🧠 Learnings (5)
📓 Common learnings
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
📚 Learning: 2025-07-16T10:40:05.717Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Applied to files:
crates/common/src/raindex_client/vaults.rs
📚 Learning: 2025-07-16T05:52:05.576Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1994
File: crates/common/src/raindex_client/vaults.rs:59-59
Timestamp: 2025-07-16T05:52:05.576Z
Learning: User findolor prefers to handle documentation updates for getter methods in batch via dedicated PRs rather than addressing them individually during feature development, as mentioned for the formatted amount string fields in crates/common/src/raindex_client/vaults.rs.
Applied to files:
crates/common/src/raindex_client/vaults.rs
📚 Learning: 2025-07-21T16:34:31.193Z
Learnt from: 0xgleb
PR: rainlanguage/rain.orderbook#1911
File: crates/subgraph/src/types/impls.rs:7-15
Timestamp: 2025-07-21T16:34:31.193Z
Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb considers breaking changes that remove unsafe default behaviors to be intentional and acceptable. Specifically, the get_decimals() method in crates/subgraph/src/types/impls.rs was intentionally changed to return MissingDecimals error instead of defaulting to 18 decimals, as defaulting to 18 is considered unsafe and should never have been done.
Applied to files:
crates/common/src/raindex_client/vaults.rs
📚 Learning: 2025-06-20T07:51:08.790Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1941
File: crates/js_api/src/raindex/vaults.rs:175-181
Timestamp: 2025-06-20T07:51:08.790Z
Learning: In the RaindexClient vault methods, `&self` parameters are intentionally kept for API consistency to make all vault operations instance methods, even when the methods don't use client state. This is a design preference for maintaining a uniform interface.
Applied to files:
crates/common/src/raindex_client/vaults.rs
🧬 Code graph analysis (1)
crates/common/src/types/token_vault_flattened.rs (1)
crates/common/src/raindex_client/vaults.rs (4)
balance(103-105)balance(145-147)balance(184-187)balance(195-197)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: test
- GitHub Check: test
- GitHub Check: Deploy-Preview-Push
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: git-clean
- GitHub Check: test
- GitHub Check: Deploy-Docs-Preview
🔇 Additional comments (3)
crates/common/src/types/token_vault_flattened.rs (2)
229-229: Test update matches new formatting behavior.Expectation updated to scientific notation is correct for high-precision input.
26-26: Confirmed downstream Rust consumers pass throughbalance_displayverbatim, supporting scientific notation.crates/common/src/raindex_client/vaults.rs (1)
1183-1184: ConfirmFloat::format()always produces fixed-decimal strings — TS tests assert exactformattedBalancevalues (e.g.'1.5','0.000000000000001') and no parseFloat/Number calls targetformattedBalance; ensure no exponent notation is ever emitted.
Comment on lines
+712
to
715
| let formatted_amount = amount.format()?; | ||
| let formatted_new_balance = new_balance.format()?; | ||
| let formatted_old_balance = old_balance.format()?; | ||
|
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use of Float::format() for amount/new/old: consistent and safer than assuming 18 decimals.
Looks good and consistent with the broader change away from format18(). Please ensure UI/consumers of formatted_* tolerate exponent form (no fixed decimal guarantee).
Consider documenting on the RaindexVaultBalanceChange fields that formatted_* may be in scientific notation to set expectations for clients.
Also applies to: 744-747
🤖 Prompt for AI Agents
In crates/common/src/raindex_client/vaults.rs around lines 712-715 (and also
update the similar code at 744-747), the code now uses Float::format() to
produce formatted_amount/formatted_new_balance/formatted_old_balance which may
emit scientific/exponent notation; ensure downstream UI/consumers tolerate
exponent form and document this behavior on the RaindexVaultBalanceChange
fields. Update the struct/comments for RaindexVaultBalanceChange to state that
formatted_* strings may be in scientific notation (no fixed decimal guarantee),
and add/adjust any nearby callers or tests to accept exponent-formatted strings
(or normalize them where needed) so the change does not break consumers.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
packages/orderbook/test/js_api/raindexClient.test.ts(6 hunks)script/DiagOrder.sol(1 hunks)tauri-app/src-tauri/tauri.conf.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
packages/orderbook/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/orderbook/**/*.{ts,tsx}: Orderbook package: build withnpm run build:orderbook
Orderbook package: type/lint check withnpm run check -w @rainlanguage/orderbook
Orderbook package: run tests withnpm run test -w @rainlanguage/orderbook
Files:
packages/orderbook/test/js_api/raindexClient.test.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
📚 Learning: 2025-05-14T17:13:13.878Z
Learnt from: 0xgleb
PR: rainlanguage/rain.orderbook#1776
File: tauri-app/src-tauri/tauri.conf.json:56-59
Timestamp: 2025-05-14T17:13:13.878Z
Learning: Tauri configuration (tauri.conf.json) does not support environment variable interpolation in framework paths for macOS. Using syntax like ${VAR}/lib.dylib will result in a literal "Library not found" error with that exact string.
Applied to files:
tauri-app/src-tauri/tauri.conf.json
📚 Learning: 2025-05-14T17:13:13.878Z
Learnt from: 0xgleb
PR: rainlanguage/rain.orderbook#1776
File: tauri-app/src-tauri/tauri.conf.json:56-59
Timestamp: 2025-05-14T17:13:13.878Z
Learning: Tauri configuration (tauri.conf.json) does not support environment variable interpolation in framework paths. Using syntax like ${VAR}/lib.dylib will result in a literal "Library not found" error with that exact string.
Applied to files:
tauri-app/src-tauri/tauri.conf.json
📚 Learning: 2025-08-04T09:07:00.160Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2048
File: packages/webapp/src/__tests__/WithdrawModal.test.ts:36-37
Timestamp: 2025-08-04T09:07:00.160Z
Learning: In packages/webapp/src/__tests__/WithdrawModal.test.ts and similar test files, findolor considers it acceptable to directly unwrap .value from Float.fromFixedDecimal() and Float.parse() calls without explicit error handling, preferring to keep test fixtures simple rather than adding comprehensive error checking.
Applied to files:
packages/orderbook/test/js_api/raindexClient.test.ts
📚 Learning: 2025-06-04T10:21:01.388Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1907
File: packages/orderbook/test/common/test.test.ts:75-77
Timestamp: 2025-06-04T10:21:01.388Z
Learning: The DotrainOrder.create API in packages/orderbook/test/common/test.test.ts is internal and not used directly in consumer applications, so API changes here don't require external breaking change documentation.
Applied to files:
packages/orderbook/test/js_api/raindexClient.test.ts
📚 Learning: 2025-09-06T07:12:25.751Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-06T07:12:25.751Z
Learning: Applies to packages/orderbook/**/*.{ts,tsx} : Orderbook package: run tests with `npm run test -w rainlanguage/orderbook`
Applied to files:
packages/orderbook/test/js_api/raindexClient.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: test
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: git-clean
- GitHub Check: test
- GitHub Check: test
- GitHub Check: Deploy-Docs-Preview
- GitHub Check: Deploy-Preview-Push
🔇 Additional comments (2)
script/DiagOrder.sol (1)
19-19: Trailing newline only — OKNo functional change.
packages/orderbook/test/js_api/raindexClient.test.ts (1)
568-568: API rename toformat(): tests updated; noformat18()remaining
Allformat18()calls have been removed and tests now use.format(). Build the package and confirm that the generated.d.ts(e.g. index.d.ts) exposes.format()on the relevant types.
Contributor
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
Contributor
|
Tip For best results, initiate chat on the files or code changes. SIZE=M 🧠 Learnings used |
Merged
4 tasks
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.
Motivation
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit