Unify working with QueryFilters and introduce FilterStores#2006
Unify working with QueryFilters and introduce FilterStores#2006
Conversation
WalkthroughThis change introduces a modular and extensible filter management system for vault queries in the SDK and JS API. It migrates filter state and logic from the UI layer into the SDK, providing Rust and WebAssembly-compatible filter store abstractions. The update adds builder patterns, persistent storage (localStorage, URL params), error handling, and comprehensive tests. Client and UI code are updated to use the new centralized filter objects. Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant JS_API as JS API (WASM)
participant FilterStore
participant LocalStorage
participant URLParams
UI->>JS_API: DefaultWebFilterStore.create(key)
JS_API->>URLParams: Try load filters from URL
alt URL params present
URLParams-->>JS_API: Return filters
else
JS_API->>LocalStorage: Try load filters from localStorage
alt localStorage present
LocalStorage-->>JS_API: Return filters
else
JS_API->>FilterStore: Use default filters
end
end
JS_API-->>UI: Return DefaultWebFilterStore instance
UI->>JS_API: store.update_vaults(builderFn)
JS_API->>FilterStore: Update filters via builder
JS_API->>LocalStorage: Save updated filters
JS_API->>URLParams: Save updated filters to URL
JS_API-->>UI: Return updated store
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Suggested labels
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
|
@brusherru couple of points, so far just responding to the description of this PR:
|
Yes, that's right. I made that abstraction to make it possible to have another implementation of persistent store if we want (e.g. fs , index db, whatever)
Yes, it can, good idea! ❓And then I guess such stores that are using web-sys should be moved inside
Not exactly. I guess we wanted to have persistent filters between app sessions or when we're visiting different pages (not only when User refreshes the page). However, changing of URLSearchParams makes it easy to share filters. But having both localStorage and URLParams really introduces some problematic edge cases... ❓ So I want to propose using only URLParams for filters, but we may introduce a separate feature called "Saved filters" to save/load presets explicitly (by User action) and avoid conflicts?
Agree. -- User flows (and problems with having both LocalStorage + URLParams):
Problems
-- @hardyjosh what do you think? |
|
I actually don't see the UX problem with the most recently modified filters being retrieved on a fresh tab. At least for now I think it will be fine for 99% of cases. In the future, I really like the idea of saved filters (power users would love this), but let's just make an issue and put it in the backlog. |
brusherru
force-pushed
the
feat-1954-query-filters
branch
from
61a874c to
4dfa4f3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locktauri-app/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
crates/common/Cargo.toml(1 hunks)crates/common/src/raindex_client/filters/mod.rs(1 hunks)crates/common/src/raindex_client/filters/store_basic.rs(1 hunks)crates/common/src/raindex_client/filters/traits.rs(1 hunks)crates/common/src/raindex_client/filters/vaults_builder.rs(1 hunks)crates/common/src/raindex_client/filters/vaults_filter.rs(1 hunks)crates/common/src/raindex_client/mod.rs(1 hunks)crates/common/src/raindex_client/vaults.rs(6 hunks)crates/js_api/Cargo.toml(1 hunks)crates/js_api/src/filters/errors.rs(1 hunks)crates/js_api/src/filters/mod.rs(1 hunks)crates/js_api/src/filters/store_default_web.rs(1 hunks)crates/js_api/src/filters/store_localstorage.rs(1 hunks)crates/js_api/src/filters/store_urlparams.rs(1 hunks)crates/js_api/src/filters/traits.rs(1 hunks)crates/js_api/src/lib.rs(1 hunks)packages/orderbook/test/js_api/filters.test.ts(1 hunks)packages/orderbook/test/js_api/raindexClient.test.ts(1 hunks)packages/ui-components/src/lib/components/tables/VaultsListTable.svelte(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
crates/js_api/src/filters/traits.rs (1)
crates/js_api/src/filters/store_localstorage.rs (2)
load(138-154)save(156-164)
crates/common/src/raindex_client/filters/traits.rs (4)
crates/common/src/raindex_client/filters/vaults_builder.rs (1)
build(63-70)crates/common/src/raindex_client/filters/store_basic.rs (3)
get_vaults(25-27)set_vaults(28-30)update_vaults(31-39)crates/js_api/src/filters/store_localstorage.rs (3)
get_vaults(118-120)set_vaults(121-123)update_vaults(125-132)crates/common/src/raindex_client/vaults.rs (1)
get_vaults(948-999)
crates/js_api/src/filters/store_default_web.rs (2)
crates/js_api/src/filters/store_urlparams.rs (7)
new(25-28)get_vaults(42-44)set_vaults(46-48)update_vaults(50-59)load(63-71)save(73-77)filters_equal(160-163)crates/js_api/src/filters/store_localstorage.rs (8)
new(57-61)new(68-70)get_vaults(118-120)set_vaults(121-123)update_vaults(125-132)load(138-154)save(156-164)filters_equal(181-184)
⏰ 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: test
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- 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-sol-static)
- 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-artifacts, true)
- GitHub Check: test
- GitHub Check: Deploy-Preview
- GitHub Check: git-clean
- GitHub Check: Deploy-Docs-Preview
- GitHub Check: test
- GitHub Check: build-tauri (ubuntu-22.04, true)
🔇 Additional comments (33)
crates/common/src/raindex_client/mod.rs (1)
29-29: LGTM! Clean module addition for filter abstractions.The addition of the
filtersmodule properly exposes the new filtering infrastructure, following standard Rust module organization patterns.crates/common/Cargo.toml (1)
45-45: LGTM! Appropriate dependency addition for error handling.Adding
anyhowas a workspace dependency is the correct approach to support error handling in the new filter store implementations.crates/js_api/src/lib.rs (1)
6-6: LGTM! Proper module addition for WebAssembly filter functionality.The conditional compilation for WASM targets is appropriate, and the module placement follows the existing pattern.
packages/orderbook/test/js_api/raindexClient.test.ts (1)
465-466: LGTM! Test correctly updated to match the refactored API.The test properly reflects the API change where
chainIdsis now part of the filter options object instead of a separate parameter, maintaining the same functional test coverage.crates/common/src/raindex_client/filters/mod.rs (1)
1-4: LGTM! Well-organized module structure for the filter system.The module organization is clean and follows Rust conventions with descriptive names that clearly indicate their purpose:
traitsfor core interfacesvaults_filterfor filter data structuresvaults_builderfor builder pattern implementationstore_basicfor basic filter store functionalitycrates/js_api/src/filters/mod.rs (1)
1-7: Well-structured module organization.The module organization follows Rust best practices by separating concerns into logical submodules and providing a clean public interface through the re-export.
crates/js_api/src/filters/traits.rs (1)
3-7: Well-designed trait with appropriate method signatures.The trait design correctly extends
FilterStoreand provides appropriate mutability for each method -loadis mutable for updating store state, whilesaveis immutable for reading current state. The consistent error handling withPersistentFilterStoreErroraligns well with the implementations.packages/ui-components/src/lib/components/tables/VaultsListTable.svelte (1)
72-73: API change aligns well with the filter consolidation.Moving
chainIdsinto the options object is consistent with the broader refactoring to consolidate all filter parameters into a singleGetVaultsFiltersstructure. This improves API consistency and maintainability.crates/js_api/Cargo.toml (1)
32-37: Dependencies appropriately chosen for filter store functionality.The added dependencies are well-selected for the new filter store implementations:
anyhowfor consistent error handling across the workspaceweb-sysfeatures precisely target the needed browser APIs (localStorage, URL parameters)serde_jsonpromotion to main dependencies is necessary for JSON serialization in persistent storescrates/common/src/raindex_client/filters/traits.rs (3)
8-13: Well-designed builder pattern trait.The
FilterBuildertrait follows standard Rust builder patterns with a genericOutputtype and clear documentation. The design provides flexibility for different filter implementations.
15-16: Correctly implemented marker trait for serializable filters.The
Filtertrait properly uses a higher-ranked trait bound (HRTB)for<'de> Deserialize<'de>to handle deserialization lifetimes correctly. This ensures all filter types can be serialized and deserialized consistently.
18-28: Excellent filter store interface design.The
FilterStoretrait provides a clean and well-documented interface for filter management. The method signatures are well-chosen:
- Value semantics for
get_vaultsandset_vaultsensure clear ownership- The
update_vaultsmethod uses a functional approach with closures, enabling immutable updates- Proper error handling with
Result<(), anyhow::Error>for persistence failurescrates/common/src/raindex_client/filters/vaults_filter.rs (4)
10-21: Well-designed filter struct with excellent TypeScript integration.The
GetVaultsFiltersstruct is properly designed with:
- Appropriate use of
Optiontypes for optional filtering criteria- Correct TypeScript bindings with custom type annotations (
Address[],ChainIds)- Camel case serialization for JavaScript compatibility
- Proper use of Ethereum address types from Alloy
23-44: Correct conversion implementation for subgraph compatibility.The
TryFromimplementation properly handles the conversion toSgVaultsListFilterArgs:
- Owner addresses are correctly converted to
SgBytes- Token addresses are appropriately lowercased (Ethereum standard)
- Optional tokens field is handled gracefully with
unwrap_or_default()
46-46: Correct marker trait implementation.The
Filtertrait implementation is appropriate as it's a marker trait that relies on theSerializeandDeserializederives already present on the struct.
48-114: Comprehensive test coverage for filter functionality.The test suite thoroughly validates all aspects of
GetVaultsFilters:
- Default value initialization
- Type conversions with real Ethereum addresses
- Chain ID filtering functionality
- Proper handling of optional fields
The tests provide good coverage and use realistic test data.
crates/common/src/raindex_client/vaults.rs (4)
5-8: Clean modularization of filter imports.The imports correctly reflect the new filter module structure, bringing in
GetVaultsFiltersfrom the dedicatedfilters::vaults_filtermodule. This improves code organization and separation of concerns.
948-965: Excellent API simplification with improved parameter consolidation.The updated
get_vaultsmethod signature is much cleaner:
- Consolidates all filtering criteria into a single
filtersparameter- Eliminates redundant
chain_idsparameter by extracting it from filters internally- Maintains backward compatibility by using
Option<GetVaultsFilters>The internal chain ID extraction logic correctly handles all edge cases with proper Option handling.
1323-1323: Test correctly updated for new API signature.The test call properly uses the simplified
get_vaults(None, None)signature, reflecting the consolidation of chain IDs into the filters parameter.
2120-2128: Proper test adaptation for consolidated filter parameters.The test correctly constructs
GetVaultsFilterswithchain_ids: None, demonstrating the new pattern where chain IDs are part of the filter object rather than a separate parameter.crates/js_api/src/filters/errors.rs (2)
1-36: Well-designed error handling for persistent filter storage.The
PersistentFilterStoreErrorenum comprehensively covers expected failure modes:
- General load/save errors with descriptive messages
- Browser-specific issues (localStorage unavailable, window not available)
- Proper initialization error handling
The trait implementations are correct:
- Clear, user-friendly error messages in
Display- Standard
Errortrait implementation- Proper WASM error conversion for JavaScript interoperability
38-51: Clean and focused error type for URL parameter validation.The
FilterErrorenum is appropriately scoped for URL parameter validation with:
- Single variant that clearly describes the error type
- Consistent error message formatting
- Standard trait implementations following Rust error handling patterns
crates/common/src/raindex_client/filters/store_basic.rs (3)
10-22: Clean and minimal basic filter store implementation.The
BasicFilterStorestruct is well-designed for in-memory filter storage:
- Simple single-field design with appropriate derives
- Constructor follows Rust conventions
- WASM compatibility through
Tsifyfor JavaScript interoperability- Proper default initialization
24-40: Excellent FilterStore trait implementation with proper immutability.The
FilterStoreimplementation correctly follows the trait contract:
get_vaults()clones to provide value semantics without side effectsset_vaults()takes ownership for clean state replacementupdate_vaults()uses a functional approach: current state → builder → closure transformation → new stateThe immutable update pattern ensures thread safety and prevents unintended side effects.
42-299: Outstanding test coverage demonstrating robust implementation.The test suite is exceptionally comprehensive, covering:
- Basic functionality (construction, default values)
- All trait methods with realistic data
- Immutability guarantees - critical for preventing side effects
- Sequential updates - ensuring state consistency
- Serialization round-trips - validating persistence compatibility
- Clone behavior - ensuring deep copying works correctly
The tests use realistic Ethereum addresses and validate all edge cases. This level of testing demonstrates high code quality and reliability.
crates/js_api/src/filters/store_default_web.rs (3)
1-14: Imports are well-organized and appropriate.The module structure follows Rust conventions with proper separation of internal and external dependencies, and correct use of conditional compilation for WASM targets.
15-46: Well-designed composite store structure with clear documentation.The layered architecture with URL params having highest priority is appropriate for web applications, enabling both link sharing and session persistence. The constructor properly handles error propagation from the underlying stores.
48-73: Clean trait implementation with proper delegation.The implementation correctly delegates all operations to the inner composite store, maintaining the layered architecture without adding unnecessary complexity.
crates/common/src/raindex_client/filters/vaults_builder.rs (1)
39-58: Well-implemented builder pattern with proper method chaining.The setter methods correctly consume and return
self, enabling fluent API usage while maintaining immutability semantics.crates/js_api/src/filters/store_urlparams.rs (2)
15-39: Well-designed URL parameter store with proper initialization.The generic design allows flexibility while the specialized constructor provides a convenient default. Loading URL parameters during construction correctly establishes the priority order.
41-60: Correct implementation with automatic URL synchronization.The implementation properly maintains consistency by saving to URL parameters after each update operation, while get/set operations correctly delegate to the inner store.
crates/js_api/src/filters/store_localstorage.rs (2)
34-46: Clever serialization design prevents misuse.The custom deserializer that always fails with a clear error message is a good pattern to prevent accidental deserialization without the required localStorage context. This ensures data integrity and proper initialization.
115-165: Consistent trait implementation with automatic persistence.The implementation correctly maintains data consistency by automatically saving to localStorage after updates, with comprehensive error handling throughout.
| /// Save filters to URL search parameters | ||
| fn save_to_url(&self) -> Result<(), PersistentFilterStoreError> { | ||
| let window = web_sys::window().ok_or(PersistentFilterStoreError::WindowNotAvailable)?; | ||
|
|
||
| // Get current search params to preserve other parameters | ||
| let search = window.location().search().map_err(|_| { | ||
| PersistentFilterStoreError::SaveError("Failed to read current URL".to_string()) | ||
| })?; | ||
| let url_params = web_sys::UrlSearchParams::new_with_str(&search).map_err(|_| { | ||
| PersistentFilterStoreError::SaveError("Invalid current URL params".to_string()) | ||
| })?; | ||
|
|
||
| // Serialize current filters to JSON | ||
| let filters_json = serde_json::to_string(&self.store.get_vaults()).map_err(|e| { | ||
| PersistentFilterStoreError::SaveError(format!("Failed to serialize filters: {}", e)) | ||
| })?; | ||
|
|
||
| // Set filters parameter | ||
| url_params.set("filters", &filters_json); | ||
|
|
||
| // Update URL using replaceState (doesn't add history entry) | ||
| let history = window.history().map_err(|_| { | ||
| PersistentFilterStoreError::SaveError("History API not available".to_string()) | ||
| })?; | ||
|
|
||
| let pathname = window.location().pathname().map_err(|_| { | ||
| PersistentFilterStoreError::SaveError("Failed to read pathname".to_string()) | ||
| })?; | ||
| let new_url = format!("{}?{}", pathname, url_params.to_string()); | ||
|
|
||
| history | ||
| .replace_state_with_url(&JsValue::NULL, "", Some(&new_url)) | ||
| .map_err(|_| { | ||
| PersistentFilterStoreError::SaveError("Failed to update URL".to_string()) | ||
| })?; | ||
|
|
||
| Ok(()) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider URL length limitations.
Large filter objects could exceed browser URL length limits (typically 2048 characters). Consider adding validation or compression for the serialized filter data.
Options to handle URL length:
- Add a check for serialized data length and fallback to localStorage-only if too large
- Implement compression for the filter JSON
- Store only essential filter properties in URL, full state in localStorage
🤖 Prompt for AI Agents
In crates/js_api/src/filters/store_urlparams.rs between lines 115 and 151, the
serialized filter JSON is directly set as a URL parameter without checking its
length, which risks exceeding browser URL length limits. To fix this, add a
validation step after serializing the filters to JSON that checks the length of
the resulting string. If it exceeds a safe threshold (e.g., 2048 characters),
implement a fallback such as storing the filters only in localStorage or
compressing the JSON before setting it in the URL. Alternatively, consider
serializing only essential filter properties for the URL and keep the full state
in localStorage to avoid URL length issues.
There was a problem hiding this comment.
Later it might be replaced with normal URLSearchParams instead of having json serialized filters as single filters param. So it's kinda temporary solution.
There was a problem hiding this comment.
@brusherru Thank you for the clarification! That makes sense - using individual URLSearchParams (like ?owners=&hideZeroBalance=false) instead of a single JSON-serialized filters parameter would indeed address the URL length concerns naturally, as individual parameters are typically much shorter and more URL-friendly.
I appreciate you explaining the architectural direction. This context helps understand that the current implementation is a stepping stone toward a more robust URL parameter structure.
✏️ Learnings added
Learnt from: brusherru
PR: rainlanguage/rain.orderbook#2006
File: crates/js_api/src/filters/store_urlparams.rs:115-151
Timestamp: 2025-07-26T05:11:50.345Z
Learning: In the rain.orderbook project's URLParamsFilterStore implementation (crates/js_api/src/filters/store_urlparams.rs), the current approach of serializing filters as a single JSON parameter in the URL (?filters={"key":"value"}) is temporary. The plan is to replace it with normal URLSearchParams structure where each filter property becomes an individual URL parameter (e.g., ?owners=&hideZeroBalance=false), which will naturally resolve URL length limitations.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1565
File: packages/webapp/src/__tests__/registryManager.test.ts:227-238
Timestamp: 2025-04-09T13:00:24.640Z
Learning: In the rain.orderbook project, error handling was added to the RegistryManager to handle localStorage exceptions, addressing part of the verification agent's suggestions. The team should also consider testing extremely long URLs that might approach browser limits.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1565
File: packages/webapp/src/__tests__/registryManager.test.ts:20-21
Timestamp: 2025-04-09T09:40:03.131Z
Learning: When mocking browser APIs like localStorage for tests, it's not always necessary to fully implement all interface properties/methods with dynamic behavior. A hardcoded value (like setting length: 0) can be sufficient when the tests don't rely on that specific property.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1975
File: crates/js_api/src/gui/state_management.rs:412-412
Timestamp: 2025-07-09T14:00:12.206Z
Learning: In crates/js_api/src/gui/state_management.rs tests, findolor prefers to keep hard-coded serialized state constants (like SERIALIZED_STATE) rather than dynamically generating them, even when it may make tests more brittle to maintain.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1975
File: crates/js_api/src/config.rs:44-45
Timestamp: 2025-07-09T07:42:48.458Z
Learning: User findolor prefers to defer code improvements and refactoring suggestions to future PRs when they are not directly related to the current PR's scope, even when the suggestions are valid improvements.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:42.275Z
Learning: For PR #1884, after each commit review, I should provide a new comment with a PR size report in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding irrelevant files such as lock files (package-lock.json, cargo.lock, etc.).
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
crates/js_api/Cargo.toml (1)
26-26: wasm-bindgen version previously reviewedThe wasm-bindgen version update to "0.2.100" was already reviewed and approved in previous comments.
crates/js_api/src/filters/store_urlparams.rs (2)
115-151: Consider URL length limitations.Large filter objects could exceed browser URL length limits. Consider validation or alternative storage strategies for large filters.
155-199: Tests could benefit from web API mocking.Current tests are limited by web API availability. Consider adding mocking for more comprehensive coverage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
crates/common/src/raindex_client/filters/mod.rs(1 hunks)crates/common/src/raindex_client/filters/store_basic.rs(1 hunks)crates/common/src/raindex_client/filters/vaults_builder.rs(1 hunks)crates/common/src/raindex_client/mod.rs(1 hunks)crates/js_api/Cargo.toml(1 hunks)crates/js_api/src/filters/mod.rs(1 hunks)crates/js_api/src/filters/store_default_web.rs(1 hunks)crates/js_api/src/filters/store_localstorage.rs(1 hunks)crates/js_api/src/filters/store_urlparams.rs(1 hunks)crates/js_api/src/filters/test_utils.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1975
File: crates/js_api/src/config.rs:44-45
Timestamp: 2025-07-09T07:42:48.458Z
Learning: User findolor prefers to defer code improvements and refactoring suggestions to future PRs when they are not directly related to the current PR's scope, even when the suggestions are valid improvements.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:42.275Z
Learning: For PR #1884, after each commit review, I should provide a new comment with a PR size report in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding irrelevant files such as lock files (package-lock.json, cargo.lock, etc.).
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
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.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:42.275Z
Learning: After each commit in PR #1884, I should provide a new comment with the PR size report in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`. The report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).
crates/js_api/src/filters/mod.rs (7)
Learnt from: findolor
PR: #2008
File: crates/common/src/raindex_client/add_orders.rs:85-86
Timestamp: 2025-07-18T10:31:05.498Z
Learning: In the rainlanguage/rain.orderbook codebase, cfg-guarded imports like #[cfg(not(target_family = "wasm"))] use super::*; in test modules are sometimes needed to fix compiler warnings, even when similar imports exist in nested sub-modules. These should not be removed as they serve a specific purpose in the compilation process.
Learnt from: findolor
PR: #1710
File: crates/quote/Cargo.toml:30-30
Timestamp: 2025-05-14T04:49:14.621Z
Learning: In the rain.orderbook repository, wasm-bindgen-utils is intentionally used as an unconditional dependency because non-wasm targets (like tests) sometimes need to use it.
Learnt from: findolor
PR: #1744
File: crates/subgraph/src/orderbook_client.rs:44-46
Timestamp: 2025-05-14T05:13:59.713Z
Learning: In the rain.orderbook project, WASM dependencies are intentionally made available in non-WASM targets to facilitate testing of WASM-related functionality, so conditional compilation guards like #[cfg(target_family = "wasm")] should not be added to imports or implementations that may be needed for tests.
Learnt from: 0xgleb
PR: #1844
File: tauri-app/src-tauri/src/commands/wallet.rs:27-28
Timestamp: 2025-05-16T17:24:34.724Z
Learning: In Rust projects, dependencies listed in the [dependencies] section of Cargo.toml are available for both application code and tests. The tokio crate with the "full" feature set includes the "test" feature needed for #[tokio::test] to work properly.
Learnt from: findolor
PR: #1715
File: crates/js_api/src/common/mod.rs:111-118
Timestamp: 2025-04-30T09:28:36.960Z
Learning: In the rain.orderbook repository, the WASM tests are already properly configured with conditional compilation using #[cfg(target_family = "wasm")] and #[cfg(not(target_family = "wasm"))], and don't require additional wasm_bindgen_test_configure!(run_in_browser) directives.
Learnt from: 0xgleb
PR: #1844
File: tauri-app/src-tauri/src/commands/wallet.rs:27-28
Timestamp: 2025-05-16T17:24:34.724Z
Learning: In Rust projects, dependencies listed in the [dependencies] section of Cargo.toml are available for both application code and tests. There's no need to duplicate them in [dev-dependencies] unless you want a different version or configuration specifically for tests.
Learnt from: findolor
PR: #1858
File: crates/subgraph/src/orderbook_client/mod.rs:54-58
Timestamp: 2025-05-19T13:40:56.080Z
Learning: The wasm_bindgen_utils crate in the Rain Orderbook project handles conditional compilation for JsValue and JsError internally, allowing impl From<Error> for JsValue to work on non-WASM targets without explicit cfg guards.
crates/js_api/src/filters/test_utils.rs (6)
Learnt from: 0xgleb
PR: #1713
File: crates/settings/src/remote/chains/mod.rs:43-226
Timestamp: 2025-05-13T20:06:22.602Z
Learning: When writing tests for collections of complex objects in Rust, prefer item-by-item comparison over direct vector comparison to get more specific error messages that pinpoint exactly which item and field has a mismatch.
Learnt from: findolor
PR: #1983
File: crates/js_api/src/gui/mod.rs:964-1199
Timestamp: 2025-07-11T06:40:49.511Z
Learning: In crates/js_api/src/gui/mod.rs tests, findolor prefers to keep large hardcoded YAML strings (like the validation test YAML) inline in the test file rather than extracting them to separate external files for maintainability purposes.
Learnt from: findolor
PR: #1975
File: crates/js_api/src/gui/state_management.rs:412-412
Timestamp: 2025-07-09T14:00:12.206Z
Learning: In crates/js_api/src/gui/state_management.rs tests, findolor prefers to keep hard-coded serialized state constants (like SERIALIZED_STATE) rather than dynamically generating them, even when it may make tests more brittle to maintain.
Learnt from: findolor
PR: #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.
Learnt from: findolor
PR: #1903
File: crates/cli/src/transaction.rs:105-105
Timestamp: 2025-06-17T17:38:21.943Z
Learning: In Rust, Vec can be compared with Vec<&str> using assert_eq! because Vec implements PartialEq<Vec> when T: PartialEq, and String implements PartialEq<&str>. This means comparisons like assert_eq!(vec_of_strings, vec!["string_literal"]) work correctly without needing to convert string literals to String.
Learnt from: findolor
PR: #1947
File: crates/common/src/raindex_client/orders.rs:98-125
Timestamp: 2025-06-24T08:46:03.368Z
Learning: In the vault merging logic in crates/common/src/raindex_client/orders.rs, optimization isn't necessary because the maximum list items are usually around 5 items. For such small datasets, the simple three-loop approach is preferred over HashMap-based optimization due to clarity and minimal performance impact.
crates/common/src/raindex_client/mod.rs (15)
Learnt from: findolor
PR: #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.
Learnt from: findolor
PR: #1947
File: crates/common/src/raindex_client/orders.rs:98-125
Timestamp: 2025-06-24T08:46:03.368Z
Learning: In the vault merging logic in crates/common/src/raindex_client/orders.rs, optimization isn't necessary because the maximum list items are usually around 5 items. For such small datasets, the simple three-loop approach is preferred over HashMap-based optimization due to clarity and minimal performance impact.
Learnt from: findolor
PR: #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 to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Learnt from: findolor
PR: #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.
Learnt from: 0xgleb
PR: #1911
File: crates/subgraph/src/utils/mod.rs:1-1
Timestamp: 2025-07-21T16:37:20.599Z
Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb prefers explicit module imports over re-exporting symbols at higher levels. Specifically for the float constants in crates/subgraph/src/utils/float.rs, they prefer using utils::float::* rather than re-exporting with pub use float::* in the utils module, as the explicit import makes it clearer what is being imported and why it's needed.
Learnt from: 0xgleb
PR: #1790
File: tauri-app/src-tauri/src/commands/vault.rs:67-67
Timestamp: 2025-05-17T15:32:28.733Z
Learning: For the PR focused on testing Tauri commands::order module, the generic type parameter R: Runtime was selectively added where needed for the PR scope, applying the changes primarily to order.rs and related files while leaving other modules like vault.rs for potential future refactoring.
Learnt from: findolor
PR: #1710
File: crates/quote/Cargo.toml:30-30
Timestamp: 2025-05-14T04:49:14.621Z
Learning: In the rain.orderbook repository, wasm-bindgen-utils is intentionally used as an unconditional dependency because non-wasm targets (like tests) sometimes need to use it.
Learnt from: findolor
PR: #1744
File: crates/subgraph/src/orderbook_client.rs:44-46
Timestamp: 2025-05-14T05:13:59.713Z
Learning: In the rain.orderbook project, WASM dependencies are intentionally made available in non-WASM targets to facilitate testing of WASM-related functionality, so conditional compilation guards like #[cfg(target_family = "wasm")] should not be added to imports or implementations that may be needed for tests.
Learnt from: findolor
PR: #1858
File: crates/subgraph/src/orderbook_client/mod.rs:54-58
Timestamp: 2025-05-19T13:40:56.080Z
Learning: The wasm_bindgen_utils crate in the Rain Orderbook project handles conditional compilation for JsValue and JsError internally, allowing impl From<Error> for JsValue to work on non-WASM targets without explicit cfg guards.
Learnt from: findolor
PR: #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: #1950
File: crates/common/src/raindex_client/transactions.rs:34-38
Timestamp: 2025-06-24T13:30:02.968Z
Learning: When using #[wasm_bindgen] on an impl block, all methods within that block must have wasm_bindgen attributes, even if they are conditionally compiled for non-WASM targets using #[cfg(not(target_family = "wasm"))]. Removing these attributes causes compiler errors because the wasm_bindgen macro expansion processes the entire impl block and expects consistent attribute usage across all methods.
Learnt from: findolor
PR: #2008
File: crates/common/src/raindex_client/add_orders.rs:85-86
Timestamp: 2025-07-18T10:31:05.498Z
Learning: In the rainlanguage/rain.orderbook codebase, cfg-guarded imports like #[cfg(not(target_family = "wasm"))] use super::*; in test modules are sometimes needed to fix compiler warnings, even when similar imports exist in nested sub-modules. These should not be removed as they serve a specific purpose in the compilation process.
Learnt from: findolor
PR: #1715
File: crates/js_api/src/common/mod.rs:111-118
Timestamp: 2025-04-30T09:28:36.960Z
Learning: In the rain.orderbook repository, the WASM tests are already properly configured with conditional compilation using #[cfg(target_family = "wasm")] and #[cfg(not(target_family = "wasm"))], and don't require additional wasm_bindgen_test_configure!(run_in_browser) directives.
Learnt from: findolor
PR: #1715
File: crates/js_api/src/common/mod.rs:15-22
Timestamp: 2025-05-14T05:52:04.270Z
Learning: The project doesn't require #[repr(transparent)] for newtype wrappers in WASM contexts such as AddOrderCalldata and RemoveOrderCalldata as the current implementation is working as expected without it.
Learnt from: findolor
PR: #1744
File: crates/js_api/src/subgraph/vault.rs:19-37
Timestamp: 2025-05-09T05:21:40.234Z
Learning: In the rain.orderbook project, the developer prefers to use tuple structs for WASM wrappers (like VaultCalldataResult, GetVaultsResult) rather than structs with named fields.
crates/common/src/raindex_client/filters/mod.rs (7)
Learnt from: findolor
PR: #1947
File: crates/common/src/raindex_client/orders.rs:98-125
Timestamp: 2025-06-24T08:46:03.368Z
Learning: In the vault merging logic in crates/common/src/raindex_client/orders.rs, optimization isn't necessary because the maximum list items are usually around 5 items. For such small datasets, the simple three-loop approach is preferred over HashMap-based optimization due to clarity and minimal performance impact.
Learnt from: 0xgleb
PR: #1790
File: tauri-app/src-tauri/src/commands/vault.rs:67-67
Timestamp: 2025-05-17T15:32:28.733Z
Learning: For the PR focused on testing Tauri commands::order module, the generic type parameter R: Runtime was selectively added where needed for the PR scope, applying the changes primarily to order.rs and related files while leaving other modules like vault.rs for potential future refactoring.
Learnt from: 0xgleb
PR: #1911
File: crates/subgraph/src/utils/mod.rs:1-1
Timestamp: 2025-07-21T16:37:20.599Z
Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb prefers explicit module imports over re-exporting symbols at higher levels. Specifically for the float constants in crates/subgraph/src/utils/float.rs, they prefer using utils::float::* rather than re-exporting with pub use float::* in the utils module, as the explicit import makes it clearer what is being imported and why it's needed.
Learnt from: findolor
PR: #2008
File: crates/common/src/raindex_client/add_orders.rs:85-86
Timestamp: 2025-07-18T10:31:05.498Z
Learning: In the rainlanguage/rain.orderbook codebase, cfg-guarded imports like #[cfg(not(target_family = "wasm"))] use super::*; in test modules are sometimes needed to fix compiler warnings, even when similar imports exist in nested sub-modules. These should not be removed as they serve a specific purpose in the compilation process.
Learnt from: findolor
PR: #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.
Learnt from: findolor
PR: #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.
Learnt from: findolor
PR: #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 to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
crates/js_api/Cargo.toml (12)
Learnt from: findolor
PR: #1744
File: crates/subgraph/src/orderbook_client.rs:44-46
Timestamp: 2025-05-14T05:13:59.713Z
Learning: In the rain.orderbook project, WASM dependencies are intentionally made available in non-WASM targets to facilitate testing of WASM-related functionality, so conditional compilation guards like #[cfg(target_family = "wasm")] should not be added to imports or implementations that may be needed for tests.
Learnt from: findolor
PR: #1710
File: crates/quote/Cargo.toml:30-30
Timestamp: 2025-05-14T04:49:14.621Z
Learning: In the rain.orderbook repository, wasm-bindgen-utils is intentionally used as an unconditional dependency because non-wasm targets (like tests) sometimes need to use it.
Learnt from: findolor
PR: #1858
File: crates/subgraph/src/orderbook_client/mod.rs:54-58
Timestamp: 2025-05-19T13:40:56.080Z
Learning: The wasm_bindgen_utils crate in the Rain Orderbook project handles conditional compilation for JsValue and JsError internally, allowing impl From<Error> for JsValue to work on non-WASM targets without explicit cfg guards.
Learnt from: findolor
PR: #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: #1715
File: crates/js_api/src/common/mod.rs:111-118
Timestamp: 2025-04-30T09:28:36.960Z
Learning: In the rain.orderbook repository, the WASM tests are already properly configured with conditional compilation using #[cfg(target_family = "wasm")] and #[cfg(not(target_family = "wasm"))], and don't require additional wasm_bindgen_test_configure!(run_in_browser) directives.
Learnt from: 0xgleb
PR: #1844
File: tauri-app/src-tauri/src/commands/wallet.rs:27-28
Timestamp: 2025-05-16T17:24:34.724Z
Learning: In Rust projects, dependencies listed in the [dependencies] section of Cargo.toml are available for both application code and tests. The tokio crate with the "full" feature set includes the "test" feature needed for #[tokio::test] to work properly.
Learnt from: findolor
PR: #1715
File: crates/js_api/src/common/mod.rs:55-59
Timestamp: 2025-05-14T05:51:50.277Z
Learning: In the Rain Orderbook project, the error handling for WASM exported functions uses WasmEncodedError which includes both the original error message and a human-readable version, making direct modification of the JsValue conversion unnecessary.
Learnt from: findolor
PR: #2008
File: crates/common/src/raindex_client/add_orders.rs:85-86
Timestamp: 2025-07-18T10:31:05.498Z
Learning: In the rainlanguage/rain.orderbook codebase, cfg-guarded imports like #[cfg(not(target_family = "wasm"))] use super::*; in test modules are sometimes needed to fix compiler warnings, even when similar imports exist in nested sub-modules. These should not be removed as they serve a specific purpose in the compilation process.
Learnt from: 0xgleb
PR: #1844
File: tauri-app/src-tauri/src/commands/wallet.rs:27-28
Timestamp: 2025-05-16T17:24:34.724Z
Learning: In Rust projects, dependencies listed in the [dependencies] section of Cargo.toml are available for both application code and tests. There's no need to duplicate them in [dev-dependencies] unless you want a different version or configuration specifically for tests.
Learnt from: findolor
PR: #1715
File: crates/js_api/src/common/mod.rs:15-22
Timestamp: 2025-05-14T05:52:04.270Z
Learning: The project doesn't require #[repr(transparent)] for newtype wrappers in WASM contexts such as AddOrderCalldata and RemoveOrderCalldata as the current implementation is working as expected without it.
Learnt from: findolor
PR: #1950
File: crates/common/src/raindex_client/transactions.rs:34-38
Timestamp: 2025-06-24T13:30:02.968Z
Learning: When using #[wasm_bindgen] on an impl block, all methods within that block must have wasm_bindgen attributes, even if they are conditionally compiled for non-WASM targets using #[cfg(not(target_family = "wasm"))]. Removing these attributes causes compiler errors because the wasm_bindgen macro expansion processes the entire impl block and expects consistent attribute usage across all methods.
Learnt from: 0xgleb
PR: #1777
File: tauri-app/src-tauri/Cargo.toml:64-64
Timestamp: 2025-05-15T22:27:49.771Z
Learning: User 0xgleb prefers to maintain the dot notation format for some Cargo.toml dependencies (e.g., rain-interpreter-eval.path = "path") rather than converting all to inline table format, as both formats are valid in Rust.
crates/common/src/raindex_client/filters/store_basic.rs (6)
Learnt from: 0xgleb
PR: #1790
File: tauri-app/src-tauri/src/commands/vault.rs:67-67
Timestamp: 2025-05-17T15:32:28.733Z
Learning: For the PR focused on testing Tauri commands::order module, the generic type parameter R: Runtime was selectively added where needed for the PR scope, applying the changes primarily to order.rs and related files while leaving other modules like vault.rs for potential future refactoring.
Learnt from: hardingjam
PR: #1494
File: packages/ui-components/src/tests/WalletProvider.test.ts:18-28
Timestamp: 2025-03-24T12:27:07.862Z
Learning: In the WalletProvider tests, verifying that setAccountContext is called with the correct store is sufficient. Additional testing of Svelte's store implementation (like subscribing to verify the store value) is unnecessary as it would just be testing the framework itself.
Learnt from: 0xgleb
PR: #1713
File: crates/settings/src/remote/chains/mod.rs:43-226
Timestamp: 2025-05-13T20:06:22.602Z
Learning: When writing tests for collections of complex objects in Rust, prefer item-by-item comparison over direct vector comparison to get more specific error messages that pinpoint exactly which item and field has a mismatch.
Learnt from: findolor
PR: #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.
Learnt from: findolor
PR: #1947
File: crates/common/src/raindex_client/orders.rs:98-125
Timestamp: 2025-06-24T08:46:03.368Z
Learning: In the vault merging logic in crates/common/src/raindex_client/orders.rs, optimization isn't necessary because the maximum list items are usually around 5 items. For such small datasets, the simple three-loop approach is preferred over HashMap-based optimization due to clarity and minimal performance impact.
Learnt from: findolor
PR: #1975
File: crates/js_api/src/gui/state_management.rs:412-412
Timestamp: 2025-07-09T14:00:12.206Z
Learning: In crates/js_api/src/gui/state_management.rs tests, findolor prefers to keep hard-coded serialized state constants (like SERIALIZED_STATE) rather than dynamically generating them, even when it may make tests more brittle to maintain.
crates/js_api/src/filters/store_urlparams.rs (14)
Learnt from: hardingjam
PR: #1565
File: packages/webapp/src/tests/registryManager.test.ts:227-238
Timestamp: 2025-04-09T13:00:24.640Z
Learning: In the rain.orderbook project, error handling was added to the RegistryManager to handle localStorage exceptions, addressing part of the verification agent's suggestions. The team should also consider testing extremely long URLs that might approach browser limits.
Learnt from: findolor
PR: #1975
File: crates/js_api/src/gui/state_management.rs:412-412
Timestamp: 2025-07-09T14:00:12.206Z
Learning: In crates/js_api/src/gui/state_management.rs tests, findolor prefers to keep hard-coded serialized state constants (like SERIALIZED_STATE) rather than dynamically generating them, even when it may make tests more brittle to maintain.
Learnt from: hardingjam
PR: #1565
File: packages/webapp/src/tests/registryManager.test.ts:20-21
Timestamp: 2025-04-09T09:40:03.131Z
Learning: When mocking browser APIs like localStorage for tests, it's not always necessary to fully implement all interface properties/methods with dynamic behavior. A hardcoded value (like setting length: 0) can be sufficient when the tests don't rely on that specific property.
Learnt from: 0xgleb
PR: #1713
File: crates/settings/src/remote/chains/mod.rs:43-226
Timestamp: 2025-05-13T20:06:22.602Z
Learning: When writing tests for collections of complex objects in Rust, prefer item-by-item comparison over direct vector comparison to get more specific error messages that pinpoint exactly which item and field has a mismatch.
Learnt from: findolor
PR: #1903
File: crates/settings/src/yaml/orderbook.rs:371-377
Timestamp: 2025-06-17T16:21:24.384Z
Learning: In crates/settings/src/yaml/orderbook.rs tests, the user findolor considers RPC ordering in Vec assertions to be intentional and not a test brittleness issue. The ordering of RPCs in tests should be preserved as specified.
Learnt from: findolor
PR: #1903
File: tauri-app/src/routes/orders/[network]-[orderHash]/page.test.ts:68-69
Timestamp: 2025-06-17T16:28:19.664Z
Learning: In the tauri-app test files, the team is comfortable using single URLs in rpcs arrays for test mocks rather than multiple URLs, even when testing multi-RPC functionality. They don't consider this a coverage gap that needs to be addressed.
Learnt from: findolor
PR: #1983
File: crates/js_api/src/gui/mod.rs:964-1199
Timestamp: 2025-07-11T06:40:49.511Z
Learning: In crates/js_api/src/gui/mod.rs tests, findolor prefers to keep large hardcoded YAML strings (like the validation test YAML) inline in the test file rather than extracting them to separate external files for maintainability purposes.
Learnt from: 0xgleb
PR: #1911
File: crates/subgraph/src/vault_balance_changes_query.rs:133-135
Timestamp: 2025-07-23T13:03:40.669Z
Learning: User 0xgleb prefers not to make changes to test code when tests are already passing, even if the code format could be standardized differently. Working test code should be left alone unless there's a functional issue.
Learnt from: findolor
PR: #2008
File: crates/common/src/raindex_client/add_orders.rs:85-86
Timestamp: 2025-07-18T10:31:05.498Z
Learning: In the rainlanguage/rain.orderbook codebase, cfg-guarded imports like #[cfg(not(target_family = "wasm"))] use super::*; in test modules are sometimes needed to fix compiler warnings, even when similar imports exist in nested sub-modules. These should not be removed as they serve a specific purpose in the compilation process.
Learnt from: 0xgleb
PR: #1844
File: tauri-app/src-tauri/src/commands/wallet.rs:29-33
Timestamp: 2025-05-16T17:26:09.529Z
Learning: When testing error cases that might produce different types of errors depending on external conditions (such as hardware presence), using unwrap_err() without further assertions can be preferred over assert!(result.is_err()) with specific error messages to avoid misleading readers about expected error details.
Learnt from: findolor
PR: #1874
File: tauri-app/src-tauri/src/transaction_status.rs:69-127
Timestamp: 2025-05-27T06:16:35.738Z
Learning: In tauri-app/src-tauri/src/transaction_status.rs, directly accessing internal RwLock state in tests using notice.0.read().unwrap() is acceptable and does not require encapsulation through public getter methods.
Learnt from: hardingjam
PR: #1565
File: packages/webapp/src/routes/deploy/layout.test.ts:12-29
Timestamp: 2025-04-07T08:18:36.473Z
Learning: In test files for this project, hardingjam prefers to use custom mocks (such as for localStorage) rather than relying on environment-provided implementations, as this allows for spying on individual methods and having precise control over implementation details for more robust testing.
Learnt from: hardingjam
PR: #1494
File: packages/ui-components/src/tests/WalletProvider.test.ts:18-28
Timestamp: 2025-03-24T12:27:07.862Z
Learning: In the WalletProvider tests, verifying that setAccountContext is called with the correct store is sufficient. Additional testing of Svelte's store implementation (like subscribing to verify the store value) is unnecessary as it would just be testing the framework itself.
Learnt from: findolor
PR: #1715
File: crates/js_api/src/common/mod.rs:111-118
Timestamp: 2025-04-30T09:28:36.960Z
Learning: In the rain.orderbook repository, the WASM tests are already properly configured with conditional compilation using #[cfg(target_family = "wasm")] and #[cfg(not(target_family = "wasm"))], and don't require additional wasm_bindgen_test_configure!(run_in_browser) directives.
crates/js_api/src/filters/store_default_web.rs (19)
Learnt from: findolor
PR: #1483
File: crates/settings/src/yaml/cache.rs:20-22
Timestamp: 2025-04-07T09:55:44.798Z
Learning: The Cache implementation in crates/settings/src/yaml/cache.rs is designed to overwrite existing entries without warning when using update methods like update_remote_network. This behavior is an intentional design choice.
Learnt from: findolor
PR: #1950
File: crates/common/src/raindex_client/transactions.rs:34-38
Timestamp: 2025-06-24T13:30:02.968Z
Learning: When using #[wasm_bindgen] on an impl block, all methods within that block must have wasm_bindgen attributes, even if they are conditionally compiled for non-WASM targets using #[cfg(not(target_family = "wasm"))]. Removing these attributes causes compiler errors because the wasm_bindgen macro expansion processes the entire impl block and expects consistent attribute usage across all methods.
Learnt from: findolor
PR: #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.
Learnt from: findolor
PR: #2008
File: crates/common/src/raindex_client/add_orders.rs:85-86
Timestamp: 2025-07-18T10:31:05.498Z
Learning: In the rainlanguage/rain.orderbook codebase, cfg-guarded imports like #[cfg(not(target_family = "wasm"))] use super::*; in test modules are sometimes needed to fix compiler warnings, even when similar imports exist in nested sub-modules. These should not be removed as they serve a specific purpose in the compilation process.
Learnt from: findolor
PR: #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.
Learnt from: 0xgleb
PR: #1713
File: crates/settings/src/remote/chains/mod.rs:43-226
Timestamp: 2025-05-13T20:06:22.602Z
Learning: When writing tests for collections of complex objects in Rust, prefer item-by-item comparison over direct vector comparison to get more specific error messages that pinpoint exactly which item and field has a mismatch.
Learnt from: findolor
PR: #1983
File: crates/js_api/src/gui/mod.rs:964-1199
Timestamp: 2025-07-11T06:40:49.511Z
Learning: In crates/js_api/src/gui/mod.rs tests, findolor prefers to keep large hardcoded YAML strings (like the validation test YAML) inline in the test file rather than extracting them to separate external files for maintainability purposes.
Learnt from: findolor
PR: #1975
File: crates/js_api/src/gui/state_management.rs:412-412
Timestamp: 2025-07-09T14:00:12.206Z
Learning: In crates/js_api/src/gui/state_management.rs tests, findolor prefers to keep hard-coded serialized state constants (like SERIALIZED_STATE) rather than dynamically generating them, even when it may make tests more brittle to maintain.
Learnt from: findolor
PR: #1996
File: packages/ui-components/src/tests/VaultIdInformation.test.ts:6-6
Timestamp: 2025-07-17T10:36:02.846Z
Learning: In packages/ui-components/src/tests/VaultIdInformation.test.ts, findolor prefers to keep exported type aliases like VaultIdInformationComponentProps in test files, even when static analysis tools flag this as discouraged practice.
Learnt from: 0xgleb
PR: #1911
File: crates/subgraph/src/vault_balance_changes_query.rs:133-135
Timestamp: 2025-07-23T13:03:40.669Z
Learning: User 0xgleb prefers not to make changes to test code when tests are already passing, even if the code format could be standardized differently. Working test code should be left alone unless there's a functional issue.
Learnt from: findolor
PR: #1938
File: crates/settings/src/yaml/orderbook.rs:180-199
Timestamp: 2025-06-18T18:24:32.049Z
Learning: In crates/settings/src/yaml/orderbook.rs, the user prefers to avoid refactoring duplicate search logic between get_orderbook_by_address and get_orderbook_by_network_key when there are only 2 functions, indicating they would consider it if more similar functions are added in the future.
Learnt from: hardingjam
PR: #1494
File: packages/ui-components/src/tests/WalletProvider.test.ts:18-28
Timestamp: 2025-03-24T12:27:07.862Z
Learning: In the WalletProvider tests, verifying that setAccountContext is called with the correct store is sufficient. Additional testing of Svelte's store implementation (like subscribing to verify the store value) is unnecessary as it would just be testing the framework itself.
Learnt from: findolor
PR: #1903
File: crates/settings/src/yaml/orderbook.rs:371-377
Timestamp: 2025-06-17T16:21:24.384Z
Learning: In crates/settings/src/yaml/orderbook.rs tests, the user findolor considers RPC ordering in Vec assertions to be intentional and not a test brittleness issue. The ordering of RPCs in tests should be preserved as specified.
Learnt from: findolor
PR: #1974
File: packages/ui-components/src/tests/DeploymentSteps.test.ts:123-126
Timestamp: 2025-07-09T12:35:45.699Z
Learning: In packages/ui-components/src/tests/DeploymentSteps.test.ts, findolor prefers to keep mock initializations (like setSelectToken) in individual test cases rather than consolidating them into shared beforeEach blocks, even when it results in duplication.
Learnt from: findolor
PR: #1715
File: crates/js_api/src/common/mod.rs:111-118
Timestamp: 2025-04-30T09:28:36.960Z
Learning: In the rain.orderbook repository, the WASM tests are already properly configured with conditional compilation using #[cfg(target_family = "wasm")] and #[cfg(not(target_family = "wasm"))], and don't require additional wasm_bindgen_test_configure!(run_in_browser) directives.
Learnt from: findolor
PR: #1744
File: crates/subgraph/src/orderbook_client.rs:44-46
Timestamp: 2025-05-14T05:13:59.713Z
Learning: In the rain.orderbook project, WASM dependencies are intentionally made available in non-WASM targets to facilitate testing of WASM-related functionality, so conditional compilation guards like #[cfg(target_family = "wasm")] should not be added to imports or implementations that may be needed for tests.
Learnt from: findolor
PR: #1710
File: crates/quote/Cargo.toml:30-30
Timestamp: 2025-05-14T04:49:14.621Z
Learning: In the rain.orderbook repository, wasm-bindgen-utils is intentionally used as an unconditional dependency because non-wasm targets (like tests) sometimes need to use it.
Learnt from: findolor
PR: #1858
File: crates/subgraph/src/orderbook_client/mod.rs:54-58
Timestamp: 2025-05-19T13:40:56.080Z
Learning: The wasm_bindgen_utils crate in the Rain Orderbook project handles conditional compilation for JsValue and JsError internally, allowing impl From<Error> for JsValue to work on non-WASM targets without explicit cfg guards.
Learnt from: findolor
PR: #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.
crates/js_api/src/filters/store_localstorage.rs (17)
Learnt from: findolor
PR: #1917
File: packages/webapp/src/lib/stores/settings.ts:30-40
Timestamp: 2025-06-11T11:41:09.591Z
Learning: In the webapp, the settings cachedWritableStore (packages/webapp/src/lib/stores/settings.ts) is loaded from localStorage once on page load and is not written back thereafter, so deserialization fallbacks do not overwrite the persisted user data.
Learnt from: 0xgleb
PR: #1911
File: crates/subgraph/src/vault_balance_changes_query.rs:133-135
Timestamp: 2025-07-23T13:03:40.669Z
Learning: User 0xgleb prefers not to make changes to test code when tests are already passing, even if the code format could be standardized differently. Working test code should be left alone unless there's a functional issue.
Learnt from: findolor
PR: #1983
File: crates/js_api/src/gui/mod.rs:964-1199
Timestamp: 2025-07-11T06:40:49.511Z
Learning: In crates/js_api/src/gui/mod.rs tests, findolor prefers to keep large hardcoded YAML strings (like the validation test YAML) inline in the test file rather than extracting them to separate external files for maintainability purposes.
Learnt from: hardingjam
PR: #1565
File: packages/webapp/src/tests/registryManager.test.ts:20-21
Timestamp: 2025-04-09T09:40:03.131Z
Learning: When mocking browser APIs like localStorage for tests, it's not always necessary to fully implement all interface properties/methods with dynamic behavior. A hardcoded value (like setting length: 0) can be sufficient when the tests don't rely on that specific property.
Learnt from: findolor
PR: #1975
File: crates/js_api/src/gui/state_management.rs:412-412
Timestamp: 2025-07-09T14:00:12.206Z
Learning: In crates/js_api/src/gui/state_management.rs tests, findolor prefers to keep hard-coded serialized state constants (like SERIALIZED_STATE) rather than dynamically generating them, even when it may make tests more brittle to maintain.
Learnt from: hardingjam
PR: #1565
File: packages/webapp/src/routes/deploy/layout.test.ts:12-29
Timestamp: 2025-04-07T08:18:36.473Z
Learning: In test files for this project, hardingjam prefers to use custom mocks (such as for localStorage) rather than relying on environment-provided implementations, as this allows for spying on individual methods and having precise control over implementation details for more robust testing.
Learnt from: hardingjam
PR: #1724
File: packages/ui-components/src/tests/ButtonDarkMode.test.ts:1-54
Timestamp: 2025-05-03T10:29:04.009Z
Learning: JSDoc comments are not considered necessary for test files in the rainlanguage/rain.orderbook repository. Test descriptions and assertions are sufficient documentation.
Learnt from: 0xgleb
PR: #1859
File: crates/quote/src/quote_debug.rs:472-492
Timestamp: 2025-05-20T10:20:08.206Z
Learning: In the Rain Orderbook codebase, the #[tokio::test(flavor = "multi_thread")] annotation is specifically needed for tests that use LocalEvm, not just for consistency across all async tests.
Learnt from: findolor
PR: #1996
File: packages/ui-components/src/tests/VaultIdInformation.test.ts:6-6
Timestamp: 2025-07-17T10:36:02.846Z
Learning: In packages/ui-components/src/tests/VaultIdInformation.test.ts, findolor prefers to keep exported type aliases like VaultIdInformationComponentProps in test files, even when static analysis tools flag this as discouraged practice.
Learnt from: findolor
PR: #1458
File: packages/orderbook/test/js_api/gui.test.ts:1069-1072
Timestamp: 2025-03-27T10:22:32.081Z
Learning: It's acceptable to use @ts-expect-error comments to bypass TypeScript type checking for WASM return values when there are type mismatches between WebAssembly outputs and TypeScript interfaces, rather than modifying the type definitions or implementing conversion layers.
Learnt from: findolor
PR: #1874
File: tauri-app/src-tauri/src/transaction_status.rs:69-127
Timestamp: 2025-05-27T06:16:35.738Z
Learning: In tauri-app/src-tauri/src/transaction_status.rs, directly accessing internal RwLock state in tests using notice.0.read().unwrap() is acceptable and does not require encapsulation through public getter methods.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:42.275Z
Learning: After each commit in PR #1884, I should provide a new comment with the PR size report in the format: TOTAL=number ADDITIONS=number DELETIONS=number. The report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: TOTAL=number ADDITIONS=number DELETIONS=number, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: TOTAL=number ADDITIONS=number DELETIONS=number, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: TOTAL=number ADDITIONS=number DELETIONS=number, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: TOTAL=number ADDITIONS=number DELETIONS=number, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:46:08.530Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report should be the only content in the comment - no text before it, no text after it, no formatting blocks, just the raw report in the exact format: "TOTAL=number\nADDITIONS=number\nDELETIONS=number". The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
crates/common/src/raindex_client/filters/vaults_builder.rs (17)
Learnt from: findolor
PR: #1744
File: crates/js_api/src/subgraph/vault.rs:19-37
Timestamp: 2025-05-09T05:21:40.234Z
Learning: In the rain.orderbook project, the developer prefers to use tuple structs for WASM wrappers (like VaultCalldataResult, GetVaultsResult) rather than structs with named fields.
Learnt from: findolor
PR: #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.
Learnt from: findolor
PR: #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 to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Learnt from: findolor
PR: #1858
File: crates/subgraph/src/orderbook_client/mod.rs:54-58
Timestamp: 2025-05-19T13:40:56.080Z
Learning: The wasm_bindgen_utils crate in the Rain Orderbook project handles conditional compilation for JsValue and JsError internally, allowing impl From<Error> for JsValue to work on non-WASM targets without explicit cfg guards.
Learnt from: findolor
PR: #2008
File: crates/common/src/raindex_client/add_orders.rs:85-86
Timestamp: 2025-07-18T10:31:05.498Z
Learning: In the rainlanguage/rain.orderbook codebase, cfg-guarded imports like #[cfg(not(target_family = "wasm"))] use super::*; in test modules are sometimes needed to fix compiler warnings, even when similar imports exist in nested sub-modules. These should not be removed as they serve a specific purpose in the compilation process.
Learnt from: findolor
PR: #1950
File: crates/common/src/raindex_client/transactions.rs:34-38
Timestamp: 2025-06-24T13:30:02.968Z
Learning: When using #[wasm_bindgen] on an impl block, all methods within that block must have wasm_bindgen attributes, even if they are conditionally compiled for non-WASM targets using #[cfg(not(target_family = "wasm"))]. Removing these attributes causes compiler errors because the wasm_bindgen macro expansion processes the entire impl block and expects consistent attribute usage across all methods.
Learnt from: findolor
PR: #1715
File: crates/js_api/src/common/mod.rs:15-22
Timestamp: 2025-05-14T05:52:04.270Z
Learning: The project doesn't require #[repr(transparent)] for newtype wrappers in WASM contexts such as AddOrderCalldata and RemoveOrderCalldata as the current implementation is working as expected without it.
Learnt from: findolor
PR: #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.
Learnt from: 0xgleb
PR: #1790
File: tauri-app/src-tauri/src/commands/vault.rs:67-67
Timestamp: 2025-05-17T15:32:28.733Z
Learning: For the PR focused on testing Tauri commands::order module, the generic type parameter R: Runtime was selectively added where needed for the PR scope, applying the changes primarily to order.rs and related files while leaving other modules like vault.rs for potential future refactoring.
Learnt from: findolor
PR: #1715
File: crates/js_api/src/common/mod.rs:55-59
Timestamp: 2025-05-14T05:51:50.277Z
Learning: In the Rain Orderbook project, the error handling for WASM exported functions uses WasmEncodedError which includes both the original error message and a human-readable version, making direct modification of the JsValue conversion unnecessary.
Learnt from: 0xgleb
PR: #1911
File: crates/subgraph/src/utils/mod.rs:1-1
Timestamp: 2025-07-21T16:37:20.599Z
Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb prefers explicit module imports over re-exporting symbols at higher levels. Specifically for the float constants in crates/subgraph/src/utils/float.rs, they prefer using utils::float::* rather than re-exporting with pub use float::* in the utils module, as the explicit import makes it clearer what is being imported and why it's needed.
Learnt from: findolor
PR: #1947
File: crates/common/src/raindex_client/orders.rs:98-125
Timestamp: 2025-06-24T08:46:03.368Z
Learning: In the vault merging logic in crates/common/src/raindex_client/orders.rs, optimization isn't necessary because the maximum list items are usually around 5 items. For such small datasets, the simple three-loop approach is preferred over HashMap-based optimization due to clarity and minimal performance impact.
Learnt from: 0xgleb
PR: #1713
File: crates/settings/src/remote/chains/mod.rs:43-226
Timestamp: 2025-05-13T20:06:22.602Z
Learning: When writing tests for collections of complex objects in Rust, prefer item-by-item comparison over direct vector comparison to get more specific error messages that pinpoint exactly which item and field has a mismatch.
Learnt from: findolor
PR: #1646
File: crates/settings/src/scenario.rs:1-1
Timestamp: 2025-04-10T11:08:33.383Z
Learning: Wildcard imports (using *) are acceptable in the rain.orderbook codebase, as per the author's preference.
Learnt from: findolor
PR: #1996
File: packages/ui-components/src/tests/VaultIdInformation.test.ts:6-6
Timestamp: 2025-07-17T10:36:02.846Z
Learning: In packages/ui-components/src/tests/VaultIdInformation.test.ts, findolor prefers to keep exported type aliases like VaultIdInformationComponentProps in test files, even when static analysis tools flag this as discouraged practice.
Learnt from: 0xgleb
PR: #1846
File: crates/quote/src/quote.rs:309-317
Timestamp: 2025-05-20T15:34:06.751Z
Learning: In Rust, when importing from the alloy crate, use alloy::hex; imports the hex macro, not a module, so it can't be consolidated with imports like use alloy::hex::ToHex; using the grouped import syntax.
Learnt from: findolor
PR: #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.
🧬 Code Graph Analysis (1)
crates/js_api/src/filters/store_localstorage.rs (4)
crates/js_api/src/filters/store_urlparams.rs (7)
new(25-28)new_with_store(33-38)get_vaults(42-44)set_vaults(46-48)update_vaults(50-59)load(63-71)save(73-77)crates/common/src/raindex_client/filters/store_basic.rs (4)
new(17-21)get_vaults(25-27)set_vaults(28-30)update_vaults(31-39)crates/common/src/raindex_client/filters/traits.rs (3)
get_vaults(20-20)set_vaults(21-21)update_vaults(25-27)crates/js_api/src/filters/traits.rs (2)
load(5-5)save(6-6)
⏰ 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: Deploy-Docs-Preview
- GitHub Check: git-clean
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: test
- GitHub Check: test
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: test
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: Deploy-Preview
🔇 Additional comments (29)
crates/common/src/raindex_client/mod.rs (2)
29-29: Clean module integrationThe addition of the
filtersmodule follows the established pattern and integrates well with the existing module structure.
37-38: Well-documented re-exportsThe comment clearly explains the purpose of the re-exports, and choosing to re-export the commonly used
VaultsFilterBuilderandVaultsFilterBuilderErrortypes provides good developer ergonomics.crates/js_api/src/filters/test_utils.rs (1)
3-5: Consider more robust error handling in test utilitiesWhile this utility serves its purpose, using
unwrap()could cause test panics instead of meaningful failure messages. Consider returning aResultor usingexpect()with descriptive messages.-pub fn filters_equal(a: &GetVaultsFilters, b: &GetVaultsFilters) -> bool { - serde_json::to_string(a).unwrap() == serde_json::to_string(b).unwrap() -} +pub fn filters_equal(a: &GetVaultsFilters, b: &GetVaultsFilters) -> bool { + let a_json = serde_json::to_string(a) + .expect("Failed to serialize filter A for comparison"); + let b_json = serde_json::to_string(b) + .expect("Failed to serialize filter B for comparison"); + a_json == b_json +}⛔ Skipped due to learnings
Learnt from: 0xgleb PR: rainlanguage/rain.orderbook#1713 File: crates/settings/src/remote/chains/mod.rs:43-226 Timestamp: 2025-05-13T20:06:22.602Z Learning: When writing tests for collections of complex objects in Rust, prefer item-by-item comparison over direct vector comparison to get more specific error messages that pinpoint exactly which item and field has a mismatch.Learnt from: 0xgleb PR: rainlanguage/rain.orderbook#1844 File: tauri-app/src-tauri/src/commands/wallet.rs:29-33 Timestamp: 2025-05-16T17:26:09.529Z Learning: When testing error cases that might produce different types of errors depending on external conditions (such as hardware presence), using `unwrap_err()` without further assertions can be preferred over `assert!(result.is_err())` with specific error messages to avoid misleading readers about expected error details.Learnt from: findolor PR: rainlanguage/rain.orderbook#1983 File: crates/js_api/src/gui/mod.rs:964-1199 Timestamp: 2025-07-11T06:40:49.511Z Learning: In crates/js_api/src/gui/mod.rs tests, findolor prefers to keep large hardcoded YAML strings (like the validation test YAML) inline in the test file rather than extracting them to separate external files for maintainability purposes.Learnt from: findolor PR: rainlanguage/rain.orderbook#1975 File: crates/js_api/src/gui/state_management.rs:412-412 Timestamp: 2025-07-09T14:00:12.206Z Learning: In crates/js_api/src/gui/state_management.rs tests, findolor prefers to keep hard-coded serialized state constants (like SERIALIZED_STATE) rather than dynamically generating them, even when it may make tests more brittle to maintain.Learnt from: 0xgleb PR: rainlanguage/rain.orderbook#1911 File: crates/common/src/raindex_client/orders.rs:720-720 Timestamp: 2025-07-21T16:34:04.947Z Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb prefers using `.unwrap()` in test code rather than `.expect()` with descriptive messages, considering the direct unwrap approach acceptable for test contexts where failures should be fast and clear.Learnt from: findolor PR: rainlanguage/rain.orderbook#1903 File: crates/settings/src/yaml/orderbook.rs:371-377 Timestamp: 2025-06-17T16:21:24.384Z Learning: In crates/settings/src/yaml/orderbook.rs tests, the user findolor considers RPC ordering in Vec<Url> assertions to be intentional and not a test brittleness issue. The ordering of RPCs in tests should be preserved as specified.Learnt from: findolor PR: rainlanguage/rain.orderbook#1996 File: packages/ui-components/src/__tests__/VaultIdInformation.test.ts:6-6 Timestamp: 2025-07-17T10:36:02.846Z Learning: In packages/ui-components/src/__tests__/VaultIdInformation.test.ts, findolor prefers to keep exported type aliases like `VaultIdInformationComponentProps` in test files, even when static analysis tools flag this as discouraged practice.Learnt from: findolor PR: rainlanguage/rain.orderbook#1874 File: tauri-app/src-tauri/src/transaction_status.rs:69-127 Timestamp: 2025-05-27T06:16:35.738Z Learning: In tauri-app/src-tauri/src/transaction_status.rs, directly accessing internal RwLock state in tests using `notice.0.read().unwrap()` is acceptable and does not require encapsulation through public getter methods.Learnt from: findolor PR: rainlanguage/rain.orderbook#1858 File: crates/subgraph/src/orderbook_client/mod.rs:54-58 Timestamp: 2025-05-19T13:40:56.080Z Learning: The `wasm_bindgen_utils` crate in the Rain Orderbook project handles conditional compilation for `JsValue` and `JsError` internally, allowing `impl From<Error> for JsValue` to work on non-WASM targets without explicit cfg guards.Learnt from: 0xgleb PR: rainlanguage/rain.orderbook#1911 File: crates/subgraph/src/vault_balance_changes_query.rs:133-135 Timestamp: 2025-07-23T13:03:40.669Z Learning: User 0xgleb prefers not to make changes to test code when tests are already passing, even if the code format could be standardized differently. Working test code should be left alone unless there's a functional issue.Learnt from: findolor PR: rainlanguage/rain.orderbook#1903 File: crates/cli/src/transaction.rs:105-105 Timestamp: 2025-06-17T17:38:21.943Z Learning: In Rust, Vec<String> can be compared with Vec<&str> using assert_eq! because Vec<T> implements PartialEq<Vec<U>> when T: PartialEq<U>, and String implements PartialEq<&str>. This means comparisons like assert_eq!(vec_of_strings, vec!["string_literal"]) work correctly without needing to convert string literals to String.crates/js_api/src/filters/mod.rs (1)
1-8: Well-organized module structureThe module organization is logical with clear separation of concerns - different store implementations, traits, errors, and utilities. Re-exporting
DefaultWebFilterStoreas the primary API entry point is a good design choice.crates/js_api/Cargo.toml (1)
32-37: Appropriate dependency additions for filter functionalityThe new dependencies correctly support the filter store implementations:
anyhowfor error handling across filter storesweb-syswith necessary features for localStorage and URL parameter manipulationserde_jsonfor filter serialization/deserializationAll dependencies use workspace versions where available, maintaining consistency.
crates/common/src/raindex_client/filters/mod.rs (1)
1-6: Clean filter module organizationThe module structure effectively organizes the filter functionality with clear separation between traits, builders, filters, and stores. Re-exporting the commonly used builder types provides good API ergonomics.
crates/common/src/raindex_client/filters/store_basic.rs (4)
10-14: LGTM!The struct definition is well-structured with appropriate trait derivations for serialization and WASM compatibility.
16-22: LGTM!Clean constructor implementation using default values.
24-40: LGTM!The FilterStore implementation correctly uses the builder pattern for immutable updates and maintains proper ownership semantics.
42-299: Excellent test coverage!The test suite is comprehensive and well-structured, covering all aspects of the BasicFilterStore including immutability, serialization, and proper trait implementations.
crates/js_api/src/filters/store_urlparams.rs (4)
1-21: LGTM!Well-structured imports and generic store design that supports flexible composition.
23-39: LGTM!Good constructor design with both convenience and flexibility options. Loading URL params on initialization ensures proper precedence.
41-60: LGTM!Proper delegation pattern with automatic URL synchronization on updates.
62-78: LGTM!Clear implementation that correctly prioritizes URL parameters while maintaining fallback behavior.
crates/js_api/src/filters/store_default_web.rs (5)
1-34: LGTM!Well-documented layered architecture with clear priority ordering. The composition pattern effectively combines URL params, localStorage, and basic filter storage.
36-78: LGTM!Clean delegation pattern for trait implementations with proper error handling.
116-165: LGTM!Excellent implementation of the builder pattern across the WASM boundary. The serialization/deserialization approach correctly handles JavaScript function callbacks.
188-195: LGTM!Good use of the extracted
filters_equaltest utility function.
196-410: Excellent test coverage!Comprehensive tests covering both Rust and WASM interfaces, including proper testing of the JavaScript builder pattern integration.
crates/js_api/src/filters/store_localstorage.rs (4)
1-21: LGTM!Well-designed generic store with proper handling of the localStorage reference.
23-46: Clever serialization design!The custom serialization correctly handles the localStorage context by only serializing the inner store and preventing incorrect deserialization.
48-113: LGTM!Good WASM wrapper design with flexible constructors and proper error handling for localStorage availability.
115-165: LGTM!Proper trait implementations with automatic persistence on updates and graceful handling of missing localStorage data.
crates/common/src/raindex_client/filters/vaults_builder.rs (6)
1-6: LGTM!Clean and specific imports without wildcards.
12-42: LGTM!Well-designed error types with user-friendly messages and proper WASM error conversion.
48-104: LGTM!Well-structured builder with clear WASM constructor methods and good documentation.
106-244: LGTM!Excellent WASM setter implementations with proper validation, error handling, and immutable chaining pattern.
246-318: LGTM!Clean Rust API with proper trait implementations and clear method naming.
320-352: LGTM!Good test coverage for builder pattern and conversions.
| @@ -0,0 +1,299 @@ | |||
| use super::{vaults_builder::VaultsFilterBuilder, vaults_filter::GetVaultsFilters}; | |||
| use crate::raindex_client::filters::traits::FilterStore; | |||
| use crate::raindex_client::*; | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Replace wildcard import with specific type.
The wildcard import reduces code clarity. Based on the usage in the file, only Tsify is needed from raindex_client.
-use crate::raindex_client::*;
+use tsify::Tsify;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In crates/common/src/raindex_client/filters/store_basic.rs at line 3, replace
the wildcard import `use crate::raindex_client::*;` with a specific import of
only the `Tsify` type. This improves code clarity by explicitly stating which
item is used from the module.
|
closing as this is superseded by #2036 |
2 participants
Motivation
It closes #1954
Solution
This code introduces some structs on top of already existing filter structs that is used in RaindexClient requests.
Let's look at this using the example of structure
GetVaultsFilters, which is used as argument ofRaindexClient::get_vaultsVaultsFilterBuilderbuild()BasicFilterStoreis another struct that keeps all possible filters, such asGetVaultsFiltersandGetOrdersFilters(at the moment this is the only filters we have, but there might be more).BasicFilterStoreimplements:PersistentFilterStore, which will havesaveandloadmethods and facade methods to BasicFilterStore's setters and updaters, while updaters should also callsavemethod under the hood. In this way,settersmight be used to override state without triggeringsavemethod (see whe we need it below), andupdatemethods to update the stateYou can take a look on some tests, but
PersistentFilterStoreis not implemented yet.So here is the example of idea (rust-like pseudocode):
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores