Conversation
Contributor
WalkthroughAdds support for configurable local database remotes: new Changes
Sequence Diagram(s)sequenceDiagram
participant Parser as OrderbookYaml
participant Remotes as LocalDbRemotesParser
participant Builder as OrderbookCfg::parse_all_from_yaml
note over Parser,Remotes #DDEBF7: YAML input (possibly multi-doc)
Parser->>Remotes: parse_all_from_yaml(documents)
Remotes-->>Parser: HashMap<key, LocalDbRemoteCfg>
Parser->>Builder: for each orderbook: resolve `local-db-remote` name
alt local-db-remote present
Builder->>Remotes: lookup by name
Remotes-->>Builder: LocalDbRemoteCfg (or error)
else absent
Builder-->>Builder: default to orderbook key (or default remote)
end
Builder-->>Caller: constructed OrderbookCfg with local_db_remote field
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (4)
💤 Files with no reviewable changes (2)
🧰 Additional context used📓 Path-based instructions (4)tauri-app/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/webapp/**/*.{svelte,ts}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/{webapp,ui-components}/**/*.{svelte,ts}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-10-19T20:24:08.629ZApplied to files:
📚 Learning: 2025-04-09T12:58:03.399ZApplied to files:
📚 Learning: 2025-04-09T13:00:24.640ZApplied to files:
⏰ 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)
🔇 Additional comments (2)
Comment |
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/cli/src/commands/order/calldata.rs (1)
408-413: Addlocal-db-remotessection totest_execute_invalid_rainlang_scriptto enable orderbook parsing.The orderbook parsing logic requires a corresponding entry in
local-db-remotesfor each orderbook. When thelocal-db-remotefield is omitted from an orderbook definition, it defaults to the orderbook's key name and attempts to find it in thelocal-db-remotesmap. If not found, parsing fails with an error.Since
test_execute_invalid_rainlang_scripthas an orderbook named "some-orderbook" but nolocal-db-remotessection, add:local-db-remotes: some-orderbook: <url>This makes it consistent with
test_executeandtest_execute_no_rpc_response, which both includelocal-db-remotesalongside orderbooks.crates/js_api/src/gui/mod.rs (1)
389-395: Docs vs behavior: short-description optional or required?Docs say
gui.short-descriptionis optional, but tests assert an error when it’s missing (Lines 1606–1619). Align either docs or parser/tests.crates/settings/src/orderbook.rs (1)
58-74: Fix premature error handling in parse_network_key to skip documents without orderbooks fieldThe current code returns an error immediately if any document lacks
orderbooksas a map, preventing it from checking later documents. Therequire_hashfunction returnsFieldErrorKind::Missingwhen the field doesn't exist andFieldErrorKind::InvalidTypewhen it exists but isn't a map. You should only error onInvalidType(field exists but wrong type); skip documents missing the field entirely.Apply this refactor:
- for document in &documents { - let document_read = document.read().map_err(|_| YamlError::ReadLockError)?; - - if let Ok(orderbooks_hash) = require_hash(&document_read, Some("orderbooks"), None) { - if let Some(orderbook_yaml) = - orderbooks_hash.get(&StrictYaml::String(orderbook_key.to_string())) - { - return require_string(orderbook_yaml, Some("network"), None) - .or_else(|_| Ok(orderbook_key.to_string())); - } - } else { - return Err(YamlError::Field { - kind: FieldErrorKind::InvalidType { - field: "orderbooks".to_string(), - expected: "a map".to_string(), - }, - location: "root".to_string(), - }); - } - } + for document in &documents { + let document_read = document.read().map_err(|_| YamlError::ReadLockError)?; + // Try to read orderbooks map; skip documents that don't define it + match require_hash(&document_read, Some("orderbooks"), None) { + Ok(orderbooks_hash) => { + if let Some(orderbook_yaml) = + orderbooks_hash.get(&StrictYaml::String(orderbook_key.to_string())) + { + return require_string(orderbook_yaml, Some("network"), None) + .or_else(|_| Ok(orderbook_key.to_string())); + } + } + // If the key exists but is the wrong type, bubble the error + Err(YamlError::Field { + kind: FieldErrorKind::InvalidType { .. }, + .. + }) => { + return Err(YamlError::Field { + kind: FieldErrorKind::InvalidType { + field: "orderbooks".to_string(), + expected: "a map".to_string(), + }, + location: "root".to_string(), + }) + } + // Missing or other not-found scenarios: continue scanning other documents + Err(_) => continue, + } + }This matches the pattern correctly used in
token.rs(lines 210-230).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
tauri-app/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
crates/cli/src/commands/local_db/sync/config.rs(1 hunks)crates/cli/src/commands/order/add.rs(1 hunks)crates/cli/src/commands/order/calldata.rs(2 hunks)crates/cli/src/commands/order/orderbook_address.rs(1 hunks)crates/cli/src/commands/words.rs(1 hunks)crates/common/src/dotrain_order.rs(6 hunks)crates/common/src/raindex_client/mod.rs(3 hunks)crates/js_api/src/gui/mod.rs(11 hunks)crates/js_api/src/gui/state_management.rs(1 hunks)crates/js_api/src/yaml/mod.rs(1 hunks)crates/settings/ARCHITECTURE.md(1 hunks)crates/settings/src/lib.rs(1 hunks)crates/settings/src/local_db_remotes.rs(1 hunks)crates/settings/src/orderbook.rs(26 hunks)crates/settings/src/test.rs(2 hunks)crates/settings/src/yaml/dotrain.rs(1 hunks)crates/settings/src/yaml/mod.rs(1 hunks)crates/settings/src/yaml/orderbook.rs(11 hunks)packages/orderbook/test/common/test.test.ts(3 hunks)packages/orderbook/test/js_api/dotrainRegistry.test.ts(2 hunks)packages/orderbook/test/js_api/gui.test.ts(5 hunks)packages/orderbook/test/js_api/orderbookYaml.test.ts(1 hunks)packages/orderbook/test/js_api/raindexClient.test.ts(1 hunks)packages/webapp/src/lib/constants.ts(1 hunks)tauri-app/src-tauri/src/commands/config.rs(3 hunks)tauri-app/src-tauri/src/commands/order.rs(1 hunks)tauri-app/src/lib/services/loadRemoteSettings.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-09-24T10:54:42.060Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2163
File: crates/cli/src/commands/order/calldata.rs:47-48
Timestamp: 2025-09-24T10:54:42.060Z
Learning: In crates/cli/src/commands/order/calldata.rs, the config_deployment variable is used both as a parameter to AddOrderArgs::new_from_deployment() and later to extract RPC information, requiring the clone() to maintain access after ownership transfer.
Applied to files:
crates/cli/src/commands/order/calldata.rs
📚 Learning: 2025-10-06T11:28:30.692Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2145
File: crates/common/src/raindex_client/local_db/query/fetch_orders/query.sql:6-7
Timestamp: 2025-10-06T11:28:30.692Z
Learning: In `crates/common/src/raindex_client/local_db/query/fetch_orders/query.sql`, the orderbook_address is currently hardcoded to '0x2f209e5b67A33B8fE96E28f24628dF6Da301c8eB' because the system only supports a single orderbook at the moment. Multiorderbook logic is not yet implemented and will be added in the future.
Applied to files:
crates/cli/src/commands/order/calldata.rscrates/cli/src/commands/order/orderbook_address.rstauri-app/src-tauri/src/commands/config.rspackages/orderbook/test/common/test.test.tscrates/cli/src/commands/order/add.rspackages/orderbook/test/js_api/orderbookYaml.test.tspackages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-07-09T14:00:12.206Z
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.
Applied to files:
crates/js_api/src/gui/state_management.rs
📚 Learning: 2025-10-06T11:13:29.956Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2123
File: crates/common/src/raindex_client/local_db/mod.rs:23-29
Timestamp: 2025-10-06T11:13:29.956Z
Learning: In `crates/common/src/raindex_client/local_db/mod.rs`, the `Default` implementation for `LocalDb` that creates an RPC client pointing to `http://localhost:4444` is acceptable because the RPC client must be explicitly configured before actual usage in production scenarios.
Applied to files:
crates/settings/src/test.rs
📚 Learning: 2025-06-18T19:24:40.518Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1938
File: crates/settings/src/yaml/orderbook.rs:185-199
Timestamp: 2025-06-18T19:24:40.518Z
Learning: In crates/settings/src/yaml/orderbook.rs, the user prefers not to refactor get_orderbook_by_network_key to handle multiple orderbooks per network key since their current architecture maintains a one-to-one mapping between orderbooks and networks. They would consider the refactoring if the system evolves to support multiple orderbooks per network in the future.
Applied to files:
crates/settings/ARCHITECTURE.mdcrates/settings/src/orderbook.rs
📚 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/common/test.test.tspackages/orderbook/test/js_api/dotrainRegistry.test.tspackages/orderbook/test/js_api/orderbookYaml.test.tspackages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-10-06T08:05:02.438Z
Learnt from: CR
PR: rainlanguage/rain.orderbook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-06T08:05:02.438Z
Learning: Applies to packages/orderbook/**/*.{test,spec}.{ts,tsx} : Orderbook tests must pass (npm run test -w rainlanguage/orderbook)
Applied to files:
packages/orderbook/test/common/test.test.ts
📚 Learning: 2025-09-01T13:35:33.688Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2116
File: crates/settings/src/orderbook.rs:569-0
Timestamp: 2025-09-01T13:35:33.688Z
Learning: In crates/settings/src/orderbook.rs, the deployment-block field is required in orderbook YAML configuration and should cause a parsing error when missing, not default to any value.
Applied to files:
crates/settings/src/orderbook.rs
📚 Learning: 2025-06-17T16:21:24.384Z
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.
Applied to files:
crates/settings/src/orderbook.rs
⏰ 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). (4)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: git-clean
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: test
🔇 Additional comments (35)
crates/cli/src/commands/order/calldata.rs (1)
197-205: No issues found; YAML structure is correct per design specification.The test data structure you flagged is intentional. The implementation includes logic to default the
local-db-remotefield to the orderbook key when omitted (seecrates/settings/src/orderbook.rslines 144–148). This is validated by the test casetest_orderbook_local_db_remote_absent_defaults_to_orderbook_key, which confirms that whenlocal-db-remotesand orderbook keys match and the field is absent, the system resolves correctly. Your test additions follow this design pattern correctly.packages/orderbook/test/js_api/gui.test.ts (4)
159-161: LGTM: Consistent test data updates for local-db-remotes.The addition of
local-db-remotesconfiguration to multiple test fixtures is consistent and follows the expected YAML structure. However, consider adding explicit test cases that verify:
- The
local-db-remotesconfiguration is correctly parsed- The URL values are accessible and properly associated with orderbooks
- Error handling when invalid URLs or missing references are provided
Also applies to: 241-243, 316-318
404-405: LGTM: Reference-based local-db-remote pattern correctly demonstrated.The reference pattern (
local-db-remote: remotepointing to a named remote inlocal-db-remotes) is correctly implemented and demonstrates the intended usage where multiple orderbooks can share a common remote URL. This aligns well with the PR objectives.Also applies to: 411-411, 417-417
159-161: Based on my verification, I need to provide a clear assessment of the review comment about test coverage for the newlocal-db-remotesfunctionality.The original review comment's concern is valid but requires clarification.
The script output confirmed that:
- Test fixture data (YAML configuration including
local-db-remotesandlocal-db-remote) has been consistently updated across multiple test files (gui.test.ts, orderbookYaml.test.ts, raindexClient.test.ts, dotrainRegistry.test.ts)- However, the search results show only the fixture data itself, not explicit test cases that validate the parsing or usage of these new fields
Test fixtures are only there to help with writing a meaningful test, and to facilitate test scenarios. A test fixture is used to set up the system state and input data needed for test execution—but the presence of test data alone does not guarantee that the functionality being added is properly tested with assertions.
The review comment correctly identifies that while test data has been updated, there's no visible evidence of explicit test cases that:
- Assert the configuration is correctly parsed from YAML
- Validate that invalid references or malformed URLs are rejected
- Verify the values are accessible through the API (if applicable)
Conclude verification: The review comment's underlying concern about potential gaps in test coverage is reasonable. Without seeing explicit test cases (it/describe blocks with assertions), we cannot confirm the new fields are validated beyond just being accepted by the parser. The suggestion to verify if these aspects are tested elsewhere (e.g., in orderbookYaml.test.ts) is appropriate.
No new critical issues found beyond what the original comment raised—the concern remains valid and should be addressed by the developer through one of the suggested approaches.
984-984: Verified: serializedState change is consistent with recent local-db-remotes configuration additions.The serialized state value update at line 984 is plausible and expected. The
dotrainconfiguration used in the test includes thelocal-db-remotesfield (line 159-160), which was recently added to the codebase per git commits showing "add local-db-remotes parsing logic" and "make local db remote required." When configuration structures change, their serialized representations necessarily change.The test properly validates the round-trip behavior: line 1021 checks that serialization produces the expected value, and line 1027 confirms deserialization works correctly with that value. Since you cannot manually verify base64-gzipped data, confirm this change by:
- Running the test suite locally and verifying the state management test passes
- Or if regenerating: call
gui.serializeState()after the setup operations and confirm it matches the expected valuetauri-app/src-tauri/src/commands/config.rs (2)
39-39: LGTM! Validation flag properly added.The
local_db_remotes: trueflag has been correctly added to both validation paths (check_settings_errorsandcheck_dotrain_with_settings_errors), enabling validation of the new local-db-remotes configuration.Also applies to: 61-61
121-123: LGTM! Test fixture extended appropriately.The test fixture now includes the
local-db-remotessection, which properly exercises the new configuration parsing and validation logic.packages/orderbook/test/js_api/orderbookYaml.test.ts (1)
25-27: LGTM! Test data extended correctly.The test YAML now includes the
local-db-remotesconfiguration block, ensuring the JavaScript API properly handles the new configuration schema.packages/orderbook/test/common/test.test.ts (2)
24-26: LGTM! Test configuration extended.The test data now includes the
local-db-remotesconfiguration, maintaining consistency with other test updates in the PR.
86-86: LGTM! Test expectations aligned with sanitization.The expected Rainlang strings have been updated to remove trailing spaces after inline comments, correctly reflecting the sanitization logic introduced in
crates/common/src/dotrain_order.rs.Also applies to: 90-90, 109-109, 113-113
crates/cli/src/commands/words.rs (1)
532-534: LGTM! Test data updated consistently.The test configuration now includes
local-db-remotes, ensuring CLI commands properly handle the extended configuration schema.tauri-app/src-tauri/src/commands/order.rs (1)
608-608: LGTM! Test expectation updated for sanitization.The expected Rainlang string now reflects the trailing whitespace sanitization introduced in the Rainlang composition methods.
packages/orderbook/test/js_api/raindexClient.test.ts (1)
48-50: LGTM! Test data demonstrates remote linkage.The test configuration properly demonstrates the new local DB remotes feature:
- Defines a named remote at the top level (
remote: http://example.com)- Links both orderbooks to this remote via
local-db-remote: remoteThis provides good test coverage for the remote resolution logic.
Also applies to: 56-56, 63-63
crates/common/src/dotrain_order.rs (3)
395-405: LGTM! Rainlang sanitization properly implemented.The sanitization logic consistently trims trailing whitespace from each line before joining with newlines. This normalization improves output consistency and aligns with the test expectation updates throughout the PR.
438-448: LGTM! Sanitization applied uniformly.The same sanitization approach has been correctly applied to both
compose_scenario_to_post_task_rainlangandcompose_deployment_to_rainlang, ensuring consistent Rainlang output formatting across all composition methods.Also applies to: 483-493
749-754: LGTM! Test expectations updated correctly.The test assertions now expect sanitized Rainlang output without trailing spaces, correctly reflecting the new composition behavior.
Also applies to: 802-803, 1496-1500
tauri-app/src/lib/services/loadRemoteSettings.ts (1)
2-2: Remote settings URL verified as accessible and valid.The commit
25b7c4a8b3c6958c7fb03811913fe47f2c9f9463exists in the rain.strategies repository, and the settings.yaml file is accessible and returns valid YAML content with network configurations (polygon, arbitrum, etc.) properly structured with RPCs, chain-IDs, and currency fields. No connectivity or content validity issues detected.crates/settings/src/lib.rs (1)
7-7: LGTM!The new public module
local_db_remotesis properly exposed and follows the existing module declaration pattern in this file.crates/js_api/src/gui/state_management.rs (1)
416-416: LGTM!The serialized state constant has been updated to reflect the new configuration schema including local-db-remotes. This approach aligns with the preferred testing pattern for this module.
Based on learnings
crates/cli/src/commands/local_db/sync/config.rs (1)
52-53: LGTM!The test YAML data has been appropriately updated to include the new
local-db-remotesconfiguration section, ensuring test coverage for the new feature.crates/js_api/src/yaml/mod.rs (1)
171-178: LGTM!The test YAML has been properly updated to demonstrate the usage of the new
local-db-remotesconfiguration and the per-orderbooklocal-db-remotereference. This provides clear examples for the new feature.crates/cli/src/commands/order/add.rs (1)
143-145: LGTM!The test dotrain YAML has been updated to include the
local-db-remotesconfiguration section, maintaining consistency with the new feature across test fixtures.packages/orderbook/test/js_api/dotrainRegistry.test.ts (2)
20-20: LGTM!Whitespace normalization for consistency in YAML formatting.
Also applies to: 25-25
35-36: LGTM!The mock settings have been properly updated to include the new
local-db-remotesconfiguration and corresponding references in orderbook entries, ensuring comprehensive test coverage for the new feature.Also applies to: 42-42, 48-48
packages/webapp/src/lib/constants.ts (1)
2-4: Commit and URLs verified successfully.The commit hash
25b7c4a8b3c6958c7fb03811913fe47f2c9f9463exists in therainlanguage/rain.strategiesrepository, and both the registry and settings.yaml URLs are accessible (HTTP/2 200 responses). No action needed.crates/settings/src/yaml/mod.rs (1)
31-31: All implementations ofValidationConfighave been properly updated.Verification confirms that both implementations (
OrderbookYamlValidationandDotrainYamlValidation) include theshould_validate_local_db_remotesmethod, and the method is actively used in the validation logic.crates/settings/src/yaml/dotrain.rs (1)
56-58: Toggle added but always disabled; confirm intended scope
should_validate_local_db_remotes()returns false. If Dotrain flows will never validate local-db-remotes (orderbook concern), fine. Otherwise expose this throughDotrainYamlValidation::full()or document why it’s off.crates/settings/src/test.rs (1)
44-46: Test helper aligned with new fieldAdding
local_db_remote: Arc::new(LocalDbRemoteCfg::default())keeps helpers compiling with the new API. LGTM.crates/common/src/raindex_client/mod.rs (1)
497-505: Fixtures updated for required local-db-remotesYAML test data now includes
local-db-remotesand per‑orderbooklocal-db-remote. This matches settings parsing changes. Good coverage.Also applies to: 568-575, 870-882
crates/cli/src/commands/order/orderbook_address.rs (1)
135-144: CLI tests updated for new YAML schemaIncluding
local-db-remotesandlocal-db-remotein test YAML keeps this command compatible. No runtime logic changes needed here.Also applies to: 139-145
crates/settings/src/orderbook.rs (1)
144-162: Local DB remote resolution and defaults look correctResolves
local-db-remotewith a sensible default to the orderbook key and clear errors when missing. Nice.crates/settings/src/yaml/orderbook.rs (3)
74-76: Validation hook wired correctly.Flag and trait method are added consistently. LGTM.
129-131: Init order is correct.Parsing local-db-remotes before orderbooks prevents unresolved references. LGTM.
470-481: YAML fixtures updated — good coverage.Fixtures include both top-level and per-orderbook fields. Consider adding a negative test for missing remote name on an orderbook to assert the expected error.
If helpful, I can add a failing-case test; confirm desired error message key so I can wire it correctly.
Also applies to: 877-898, 988-1009
crates/settings/src/local_db_remotes.rs (1)
91-99: Default looks fine.Static URL parse on a constant; safe to unwrap. LGTM.
Comment on lines
+50
to
+55
| let remote_key = key_yaml.as_str().unwrap_or_default().to_string(); | ||
| let location = format!("local-db-remotes[{}]", remote_key); | ||
|
|
||
| let url_str = require_string(remote_yaml, None, Some(location.clone()))?; | ||
| let url = | ||
| LocalDbRemoteCfg::validate_url(&url_str).map_err(|e| YamlError::Field { |
Contributor
There was a problem hiding this comment.
Don’t coerce non-string keys to "" — return a type error.
Using as_str().unwrap_or_default() silently accepts non-string keys and can misattribute/duplicate entries.
Apply:
- let remote_key = key_yaml.as_str().unwrap_or_default().to_string();
+ let remote_key = key_yaml.as_str().ok_or(YamlError::Field {
+ kind: FieldErrorKind::InvalidType {
+ field: "key".to_string(),
+ expected: "a string".to_string(),
+ },
+ location: "local-db-remotes".to_string(),
+ })?.to_string();🤖 Prompt for AI Agents
crates/settings/src/local_db_remotes.rs around lines 50 to 55: the code
currently uses key_yaml.as_str().unwrap_or_default() which silently coerces
non-string YAML keys to an empty string; instead detect non-string keys and
return a Yaml type error. Replace the unwrap_or_default usage with an explicit
check (key_yaml.as_str().ok_or(...)?), constructing and returning an appropriate
YamlError::Field (including the location string like "local-db-remotes[...]" and
a message indicating the key must be a string) so invalid key types fail instead
of becoming "".
Comment on lines
+107
to
+212
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| use crate::yaml::tests::get_document; | ||
|
|
||
| #[test] | ||
| fn test_parse_local_db_remotes_from_yaml_multiple_files() { | ||
| let yaml_one = r#" | ||
| local-db-remotes: | ||
| mainnet: https://example.com/localdb/mainnet | ||
| "#; | ||
| let yaml_two = r#" | ||
| local-db-remotes: | ||
| polygon: https://example.com/localdb/polygon | ||
| "#; | ||
|
|
||
| let remotes = LocalDbRemoteCfg::parse_all_from_yaml( | ||
| vec![get_document(yaml_one), get_document(yaml_two)], | ||
| None, | ||
| ) | ||
| .unwrap(); | ||
|
|
||
| assert_eq!(remotes.len(), 2); | ||
| assert_eq!( | ||
| remotes.get("mainnet").unwrap().url, | ||
| Url::parse("https://example.com/localdb/mainnet").unwrap() | ||
| ); | ||
| assert_eq!( | ||
| remotes.get("polygon").unwrap().url, | ||
| Url::parse("https://example.com/localdb/polygon").unwrap() | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_parse_local_db_remotes_from_yaml_duplicate_key() { | ||
| let yaml_one = r#" | ||
| local-db-remotes: | ||
| mainnet: https://example.com/localdb/mainnet | ||
| "#; | ||
| let yaml_two = r#" | ||
| local-db-remotes: | ||
| mainnet: https://example.com/localdb/another | ||
| "#; | ||
| let err = LocalDbRemoteCfg::parse_all_from_yaml( | ||
| vec![get_document(yaml_one), get_document(yaml_two)], | ||
| None, | ||
| ) | ||
| .unwrap_err(); | ||
| assert_eq!( | ||
| err, | ||
| YamlError::KeyShadowing("mainnet".to_string(), "local-db-remotes".to_string()) | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_parse_local_db_remotes_optional_absent() { | ||
| // No local-db-remotes key | ||
| let yaml = r#" | ||
| test: test | ||
| "#; | ||
| let err = | ||
| LocalDbRemoteCfg::parse_all_from_yaml(vec![get_document(yaml)], None).unwrap_err(); | ||
| assert_eq!( | ||
| err, | ||
| YamlError::Field { | ||
| kind: FieldErrorKind::Missing("local-db-remotes".to_string()), | ||
| location: "root".to_string(), | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_parse_local_db_remotes_invalid_values() { | ||
| let yaml = r#" | ||
| local-db-remotes: | ||
| mainnet: | ||
| - https://example.com/localdb/mainnet | ||
| "#; | ||
| let err = | ||
| LocalDbRemoteCfg::parse_all_from_yaml(vec![get_document(yaml)], None).unwrap_err(); | ||
| assert_eq!( | ||
| err, | ||
| YamlError::Field { | ||
| kind: FieldErrorKind::InvalidType { | ||
| field: "value".to_string(), | ||
| expected: "a string".to_string(), | ||
| }, | ||
| location: "local-db-remotes[mainnet]".to_string(), | ||
| } | ||
| ); | ||
|
|
||
| let yaml = r#" | ||
| local-db-remotes: | ||
| mainnet: not_a_valid_url | ||
| "#; | ||
| let err = | ||
| LocalDbRemoteCfg::parse_all_from_yaml(vec![get_document(yaml)], None).unwrap_err(); | ||
| match err { | ||
| YamlError::Field { kind, location } => { | ||
| assert_eq!(location, "local-db-remotes[mainnet]".to_string()); | ||
| matches!(kind, FieldErrorKind::InvalidValue { .. }); | ||
| } | ||
| _ => panic!("unexpected error type"), | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add a test for non‑string keys.
Covers the fixed parser behavior when a YAML key isn’t a string.
Append:
@@
#[test]
fn test_parse_local_db_remotes_invalid_values() {
@@
}
+
+ #[test]
+ fn test_parse_local_db_remotes_non_string_key() {
+ // numeric key is not allowed; keys must be strings
+ let yaml = r#"
+local-db-remotes:
+ 123: https://example.com/localdb/num
+"#;
+ let err =
+ LocalDbRemoteCfg::parse_all_from_yaml(vec![get_document(yaml)], None).unwrap_err();
+ assert_eq!(
+ err,
+ YamlError::Field {
+ kind: FieldErrorKind::InvalidType {
+ field: "key".to_string(),
+ expected: "a string".to_string(),
+ },
+ location: "local-db-remotes".to_string(),
+ }
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #[cfg(test)] | |
| mod tests { | |
| use super::*; | |
| use crate::yaml::tests::get_document; | |
| #[test] | |
| fn test_parse_local_db_remotes_from_yaml_multiple_files() { | |
| let yaml_one = r#" | |
| local-db-remotes: | |
| mainnet: https://example.com/localdb/mainnet | |
| "#; | |
| let yaml_two = r#" | |
| local-db-remotes: | |
| polygon: https://example.com/localdb/polygon | |
| "#; | |
| let remotes = LocalDbRemoteCfg::parse_all_from_yaml( | |
| vec![get_document(yaml_one), get_document(yaml_two)], | |
| None, | |
| ) | |
| .unwrap(); | |
| assert_eq!(remotes.len(), 2); | |
| assert_eq!( | |
| remotes.get("mainnet").unwrap().url, | |
| Url::parse("https://example.com/localdb/mainnet").unwrap() | |
| ); | |
| assert_eq!( | |
| remotes.get("polygon").unwrap().url, | |
| Url::parse("https://example.com/localdb/polygon").unwrap() | |
| ); | |
| } | |
| #[test] | |
| fn test_parse_local_db_remotes_from_yaml_duplicate_key() { | |
| let yaml_one = r#" | |
| local-db-remotes: | |
| mainnet: https://example.com/localdb/mainnet | |
| "#; | |
| let yaml_two = r#" | |
| local-db-remotes: | |
| mainnet: https://example.com/localdb/another | |
| "#; | |
| let err = LocalDbRemoteCfg::parse_all_from_yaml( | |
| vec![get_document(yaml_one), get_document(yaml_two)], | |
| None, | |
| ) | |
| .unwrap_err(); | |
| assert_eq!( | |
| err, | |
| YamlError::KeyShadowing("mainnet".to_string(), "local-db-remotes".to_string()) | |
| ); | |
| } | |
| #[test] | |
| fn test_parse_local_db_remotes_optional_absent() { | |
| // No local-db-remotes key | |
| let yaml = r#" | |
| test: test | |
| "#; | |
| let err = | |
| LocalDbRemoteCfg::parse_all_from_yaml(vec![get_document(yaml)], None).unwrap_err(); | |
| assert_eq!( | |
| err, | |
| YamlError::Field { | |
| kind: FieldErrorKind::Missing("local-db-remotes".to_string()), | |
| location: "root".to_string(), | |
| } | |
| ); | |
| } | |
| #[test] | |
| fn test_parse_local_db_remotes_invalid_values() { | |
| let yaml = r#" | |
| local-db-remotes: | |
| mainnet: | |
| - https://example.com/localdb/mainnet | |
| "#; | |
| let err = | |
| LocalDbRemoteCfg::parse_all_from_yaml(vec![get_document(yaml)], None).unwrap_err(); | |
| assert_eq!( | |
| err, | |
| YamlError::Field { | |
| kind: FieldErrorKind::InvalidType { | |
| field: "value".to_string(), | |
| expected: "a string".to_string(), | |
| }, | |
| location: "local-db-remotes[mainnet]".to_string(), | |
| } | |
| ); | |
| let yaml = r#" | |
| local-db-remotes: | |
| mainnet: not_a_valid_url | |
| "#; | |
| let err = | |
| LocalDbRemoteCfg::parse_all_from_yaml(vec![get_document(yaml)], None).unwrap_err(); | |
| match err { | |
| YamlError::Field { kind, location } => { | |
| assert_eq!(location, "local-db-remotes[mainnet]".to_string()); | |
| matches!(kind, FieldErrorKind::InvalidValue { .. }); | |
| } | |
| _ => panic!("unexpected error type"), | |
| } | |
| } | |
| } | |
| #[cfg(test)] | |
| mod tests { | |
| use super::*; | |
| use crate::yaml::tests::get_document; | |
| #[test] | |
| fn test_parse_local_db_remotes_from_yaml_multiple_files() { | |
| let yaml_one = r#" | |
| local-db-remotes: | |
| mainnet: https://example.com/localdb/mainnet | |
| "#; | |
| let yaml_two = r#" | |
| local-db-remotes: | |
| polygon: https://example.com/localdb/polygon | |
| "#; | |
| let remotes = LocalDbRemoteCfg::parse_all_from_yaml( | |
| vec![get_document(yaml_one), get_document(yaml_two)], | |
| None, | |
| ) | |
| .unwrap(); | |
| assert_eq!(remotes.len(), 2); | |
| assert_eq!( | |
| remotes.get("mainnet").unwrap().url, | |
| Url::parse("https://example.com/localdb/mainnet").unwrap() | |
| ); | |
| assert_eq!( | |
| remotes.get("polygon").unwrap().url, | |
| Url::parse("https://example.com/localdb/polygon").unwrap() | |
| ); | |
| } | |
| #[test] | |
| fn test_parse_local_db_remotes_from_yaml_duplicate_key() { | |
| let yaml_one = r#" | |
| local-db-remotes: | |
| mainnet: https://example.com/localdb/mainnet | |
| "#; | |
| let yaml_two = r#" | |
| local-db-remotes: | |
| mainnet: https://example.com/localdb/another | |
| "#; | |
| let err = LocalDbRemoteCfg::parse_all_from_yaml( | |
| vec![get_document(yaml_one), get_document(yaml_two)], | |
| None, | |
| ) | |
| .unwrap_err(); | |
| assert_eq!( | |
| err, | |
| YamlError::KeyShadowing("mainnet".to_string(), "local-db-remotes".to_string()) | |
| ); | |
| } | |
| #[test] | |
| fn test_parse_local_db_remotes_optional_absent() { | |
| // No local-db-remotes key | |
| let yaml = r#" | |
| test: test | |
| "#; | |
| let err = | |
| LocalDbRemoteCfg::parse_all_from_yaml(vec![get_document(yaml)], None).unwrap_err(); | |
| assert_eq!( | |
| err, | |
| YamlError::Field { | |
| kind: FieldErrorKind::Missing("local-db-remotes".to_string()), | |
| location: "root".to_string(), | |
| } | |
| ); | |
| } | |
| #[test] | |
| fn test_parse_local_db_remotes_invalid_values() { | |
| let yaml = r#" | |
| local-db-remotes: | |
| mainnet: | |
| - https://example.com/localdb/mainnet | |
| "#; | |
| let err = | |
| LocalDbRemoteCfg::parse_all_from_yaml(vec![get_document(yaml)], None).unwrap_err(); | |
| assert_eq!( | |
| err, | |
| YamlError::Field { | |
| kind: FieldErrorKind::InvalidType { | |
| field: "value".to_string(), | |
| expected: "a string".to_string(), | |
| }, | |
| location: "local-db-remotes[mainnet]".to_string(), | |
| } | |
| ); | |
| let yaml = r#" | |
| local-db-remotes: | |
| mainnet: not_a_valid_url | |
| "#; | |
| let err = | |
| LocalDbRemoteCfg::parse_all_from_yaml(vec![get_document(yaml)], None).unwrap_err(); | |
| match err { | |
| YamlError::Field { kind, location } => { | |
| assert_eq!(location, "local-db-remotes[mainnet]".to_string()); | |
| matches!(kind, FieldErrorKind::InvalidValue { .. }); | |
| } | |
| _ => panic!("unexpected error type"), | |
| } | |
| } | |
| #[test] | |
| fn test_parse_local_db_remotes_non_string_key() { | |
| // numeric key is not allowed; keys must be strings | |
| let yaml = r#" | |
| local-db-remotes: | |
| 123: https://example.com/localdb/num | |
| "#; | |
| let err = | |
| LocalDbRemoteCfg::parse_all_from_yaml(vec![get_document(yaml)], None).unwrap_err(); | |
| assert_eq!( | |
| err, | |
| YamlError::Field { | |
| kind: FieldErrorKind::InvalidType { | |
| field: "key".to_string(), | |
| expected: "a string".to_string(), | |
| }, | |
| location: "local-db-remotes".to_string(), | |
| } | |
| ); | |
| } | |
| } |
Collaborator
Author
There was a problem hiding this comment.
everything is a string with strict yaml implementation so this is valid
Contributor
There was a problem hiding this comment.
@findolor, thank you for the clarification! You're absolutely right—with strict_yaml_rust, numeric keys like 123 are automatically coerced to strings, so they would be valid and wouldn't trigger an error. The suggested test wouldn't actually cover an error case. I appreciate you pointing this out.
✏️ Learnings added
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2270
File: crates/settings/src/local_db_remotes.rs:107-212
Timestamp: 2025-10-27T09:17:38.113Z
Learning: In the rain.orderbook codebase using strict_yaml_rust, all YAML keys are treated as strings, including numeric literals. Numeric keys like `123` are automatically coerced to strings and are valid, so they don't need error-case testing for non-string keys.
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.
4 tasks
0xgleb
approved these changes
Oct 27, 2025
Comment on lines
+39
to
+87
| let mut remotes = HashMap::new(); | ||
|
|
||
| for document in documents { | ||
| let document_read = document.read().map_err(|_| YamlError::ReadLockError)?; | ||
|
|
||
| if let Ok(remotes_hash) = require_hash( | ||
| &document_read, | ||
| Some("local-db-remotes"), | ||
| Some("root".to_string()), | ||
| ) { | ||
| for (key_yaml, remote_yaml) in remotes_hash { | ||
| let remote_key = key_yaml | ||
| .as_str() | ||
| .ok_or(YamlError::Field { | ||
| kind: FieldErrorKind::InvalidType { | ||
| field: "key".to_string(), | ||
| expected: "a string".to_string(), | ||
| }, | ||
| location: "local-db-remotes".to_string(), | ||
| })? | ||
| .to_string(); | ||
| let location = format!("local-db-remotes[{}]", remote_key); | ||
|
|
||
| let url_str = require_string(remote_yaml, None, Some(location.clone()))?; | ||
| let url = | ||
| LocalDbRemoteCfg::validate_url(&url_str).map_err(|e| YamlError::Field { | ||
| kind: FieldErrorKind::InvalidValue { | ||
| field: "url".to_string(), | ||
| reason: e.to_string(), | ||
| }, | ||
| location: location.clone(), | ||
| })?; | ||
|
|
||
| let remote = LocalDbRemoteCfg { | ||
| document: document.clone(), | ||
| key: remote_key.clone(), | ||
| url, | ||
| }; | ||
|
|
||
| if remotes.contains_key(&remote_key) { | ||
| return Err(YamlError::KeyShadowing( | ||
| remote_key, | ||
| "local-db-remotes".to_string(), | ||
| )); | ||
| } | ||
| remotes.insert(remote.key.clone(), remote); | ||
| } | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
Suggested change
| let mut remotes = HashMap::new(); | |
| for document in documents { | |
| let document_read = document.read().map_err(|_| YamlError::ReadLockError)?; | |
| if let Ok(remotes_hash) = require_hash( | |
| &document_read, | |
| Some("local-db-remotes"), | |
| Some("root".to_string()), | |
| ) { | |
| for (key_yaml, remote_yaml) in remotes_hash { | |
| let remote_key = key_yaml | |
| .as_str() | |
| .ok_or(YamlError::Field { | |
| kind: FieldErrorKind::InvalidType { | |
| field: "key".to_string(), | |
| expected: "a string".to_string(), | |
| }, | |
| location: "local-db-remotes".to_string(), | |
| })? | |
| .to_string(); | |
| let location = format!("local-db-remotes[{}]", remote_key); | |
| let url_str = require_string(remote_yaml, None, Some(location.clone()))?; | |
| let url = | |
| LocalDbRemoteCfg::validate_url(&url_str).map_err(|e| YamlError::Field { | |
| kind: FieldErrorKind::InvalidValue { | |
| field: "url".to_string(), | |
| reason: e.to_string(), | |
| }, | |
| location: location.clone(), | |
| })?; | |
| let remote = LocalDbRemoteCfg { | |
| document: document.clone(), | |
| key: remote_key.clone(), | |
| url, | |
| }; | |
| if remotes.contains_key(&remote_key) { | |
| return Err(YamlError::KeyShadowing( | |
| remote_key, | |
| "local-db-remotes".to_string(), | |
| )); | |
| } | |
| remotes.insert(remote.key.clone(), remote); | |
| } | |
| } | |
| } | |
| let remotes = documents | |
| .iter() | |
| .try_fold(HashMap::new(), |remotes, document| { | |
| let document_read = document.read().map_err(|_| YamlError::ReadLockError)?; | |
| let remotes_hash = match require_hash( | |
| &document_read, | |
| Some("local-db-remotes"), | |
| Some("root".to_string()), | |
| ) { | |
| Ok(hash) => hash, | |
| Err(_) => return Ok(remotes), | |
| }; | |
| remotes_hash.into_iter().try_fold(remotes, |mut remotes, (key_yaml, remote_yaml)| { | |
| let remote_key = key_yaml | |
| .as_str() | |
| .ok_or(YamlError::Field { | |
| kind: FieldErrorKind::InvalidType { | |
| field: "key".to_string(), | |
| expected: "a string".to_string(), | |
| }, | |
| location: "local-db-remotes".to_string(), | |
| })? | |
| .to_string(); | |
| if remotes.contains_key(&remote_key) { | |
| return Err(YamlError::KeyShadowing( | |
| remote_key, | |
| "local-db-remotes".to_string(), | |
| )); | |
| } | |
| let location = format!("local-db-remotes[{remote_key}]"); | |
| let url_str = require_string(remote_yaml, None, Some(location.clone()))?; | |
| let url = LocalDbRemoteCfg::validate_url(&url_str).map_err(|e| YamlError::Field { | |
| kind: FieldErrorKind::InvalidValue { | |
| field: "url".to_string(), | |
| reason: e.to_string(), | |
| }, | |
| location: location.clone(), | |
| })?; | |
| remotes.insert( | |
| remote_key.clone(), | |
| LocalDbRemoteCfg { | |
| document: document.clone(), | |
| key: remote_key, | |
| url, | |
| }, | |
| ); | |
| Ok(remotes) | |
| }) | |
| })?; |
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 (8)
crates/cli/src/commands/order/calldata.rs(2 hunks)crates/common/src/raindex_client/mod.rs(3 hunks)crates/settings/ARCHITECTURE.md(1 hunks)crates/settings/src/local_db_remotes.rs(1 hunks)crates/settings/src/yaml/orderbook.rs(12 hunks)packages/orderbook/test/common/test.test.ts(3 hunks)packages/orderbook/test/js_api/gui.test.ts(5 hunks)packages/orderbook/test/js_api/raindexClient.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
crates/*/{src,tests,benches,examples}/**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Rust code must pass clippy with all warnings denied (cargo clippy --workspace --all-targets --all-features -D warnings)
Files:
crates/common/src/raindex_client/mod.rscrates/settings/src/yaml/orderbook.rscrates/cli/src/commands/order/calldata.rscrates/settings/src/local_db_remotes.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Rust code lives in the workspace under crates/* (e.g., cli, common, bindings, js_api, quote, subgraph, settings, math, integration_tests)
Files:
crates/common/src/raindex_client/mod.rscrates/settings/src/yaml/orderbook.rscrates/cli/src/commands/order/calldata.rscrates/settings/src/local_db_remotes.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust with nix develop -c cargo fmt --all
Lint Rust with nix develop -c rainix-rs-static (preconfigured flags)
Use snake_case for Rust crates/modules and PascalCase for types
Files:
crates/common/src/raindex_client/mod.rscrates/settings/src/yaml/orderbook.rscrates/cli/src/commands/order/calldata.rscrates/settings/src/local_db_remotes.rs
packages/orderbook/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Orderbook TypeScript package must pass type/lint checks (npm run check -w @rainlanguage/orderbook)
Files:
packages/orderbook/test/common/test.test.tspackages/orderbook/test/js_api/raindexClient.test.tspackages/orderbook/test/js_api/gui.test.ts
packages/orderbook/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Orderbook tests must pass (npm run test -w @rainlanguage/orderbook)
Files:
packages/orderbook/test/common/test.test.tspackages/orderbook/test/js_api/raindexClient.test.tspackages/orderbook/test/js_api/gui.test.ts
packages/**
📄 CodeRabbit inference engine (AGENTS.md)
JavaScript/Svelte projects live under packages/* (webapp, ui-components, orderbook)
Files:
packages/orderbook/test/common/test.test.tspackages/orderbook/test/js_api/raindexClient.test.tspackages/orderbook/test/js_api/gui.test.ts
packages/**/*.{test,spec}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name TypeScript test files as *.test.ts or *.spec.ts (Vitest)
Files:
packages/orderbook/test/common/test.test.tspackages/orderbook/test/js_api/raindexClient.test.tspackages/orderbook/test/js_api/gui.test.ts
🧠 Learnings (65)
📓 Common learnings
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the format: "TOTAL=number ADDITIONS=number DELETIONS=number". This report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 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).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the exact format: "TOTAL=number\nADDITIONS=number\nDELETIONS=number". This report should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: findolor
Repo: rainlanguage/rain.orderbook 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.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2145
File: crates/common/src/raindex_client/local_db/query/create_tables/query.sql:71-72
Timestamp: 2025-10-06T11:44:07.888Z
Learning: The local DB feature in the rain.orderbook codebase is not live yet (as of PR #2145), so schema migrations for existing databases are not required when modifying table structures in `crates/common/src/raindex_client/local_db/query/create_tables/query.sql`.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2237
File: crates/common/src/raindex_client/local_db/sync.rs:79-89
Timestamp: 2025-10-18T10:38:41.273Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the sync_database method currently only supports indexing a single orderbook per chain ID, which is why `.first()` is used to select the orderbook configuration. Multi-orderbook support per chain ID is planned for future PRs.
📚 Learning: 2025-10-14T07:51:55.148Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2202
File: crates/common/src/raindex_client/local_db/sync.rs:33-34
Timestamp: 2025-10-14T07:51:55.148Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the hard-coded `DEFAULT_SYNC_CHAIN_ID` constant (set to `SUPPORTED_LOCAL_DB_CHAINS[0]`) will be replaced with proper chain ID handling in downstream PRs as part of the multi-network/orderbook implementation.
Applied to files:
crates/common/src/raindex_client/mod.rspackages/orderbook/test/js_api/raindexClient.test.tscrates/settings/src/yaml/orderbook.rscrates/cli/src/commands/order/calldata.rscrates/settings/ARCHITECTURE.md
📚 Learning: 2025-10-06T11:44:07.888Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2145
File: crates/common/src/raindex_client/local_db/query/create_tables/query.sql:71-72
Timestamp: 2025-10-06T11:44:07.888Z
Learning: The local DB feature in the rain.orderbook codebase is not live yet (as of PR #2145), so schema migrations for existing databases are not required when modifying table structures in `crates/common/src/raindex_client/local_db/query/create_tables/query.sql`.
Applied to files:
crates/common/src/raindex_client/mod.rspackages/orderbook/test/js_api/raindexClient.test.tscrates/settings/src/yaml/orderbook.rscrates/cli/src/commands/order/calldata.rs
📚 Learning: 2025-10-18T10:38:41.273Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2237
File: crates/common/src/raindex_client/local_db/sync.rs:79-89
Timestamp: 2025-10-18T10:38:41.273Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the sync_database method currently only supports indexing a single orderbook per chain ID, which is why `.first()` is used to select the orderbook configuration. Multi-orderbook support per chain ID is planned for future PRs.
Applied to files:
crates/common/src/raindex_client/mod.rspackages/orderbook/test/common/test.test.tspackages/orderbook/test/js_api/raindexClient.test.tspackages/orderbook/test/js_api/gui.test.tscrates/settings/src/yaml/orderbook.rscrates/cli/src/commands/order/calldata.rscrates/settings/ARCHITECTURE.md
📚 Learning: 2025-10-06T11:13:29.956Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2123
File: crates/common/src/raindex_client/local_db/mod.rs:23-29
Timestamp: 2025-10-06T11:13:29.956Z
Learning: In `crates/common/src/raindex_client/local_db/mod.rs`, the `Default` implementation for `LocalDb` that creates an RPC client pointing to `http://localhost:4444` is acceptable because the RPC client must be explicitly configured before actual usage in production scenarios.
Applied to files:
crates/common/src/raindex_client/mod.rscrates/settings/src/local_db_remotes.rscrates/settings/ARCHITECTURE.md
📚 Learning: 2025-06-18T19:24:40.518Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1938
File: crates/settings/src/yaml/orderbook.rs:185-199
Timestamp: 2025-06-18T19:24:40.518Z
Learning: In crates/settings/src/yaml/orderbook.rs, the user prefers not to refactor get_orderbook_by_network_key to handle multiple orderbooks per network key since their current architecture maintains a one-to-one mapping between orderbooks and networks. They would consider the refactoring if the system evolves to support multiple orderbooks per network in the future.
Applied to files:
crates/common/src/raindex_client/mod.rspackages/orderbook/test/common/test.test.tspackages/orderbook/test/js_api/gui.test.tscrates/settings/src/yaml/orderbook.rscrates/cli/src/commands/order/calldata.rscrates/settings/ARCHITECTURE.md
📚 Learning: 2025-05-21T21:24:53.708Z
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 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.
Applied to files:
crates/common/src/raindex_client/mod.rscrates/cli/src/commands/order/calldata.rs
📚 Learning: 2025-06-17T16:21:24.384Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook 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<Url> assertions to be intentional and not a test brittleness issue. The ordering of RPCs in tests should be preserved as specified.
Applied to files:
crates/common/src/raindex_client/mod.rscrates/settings/src/yaml/orderbook.rscrates/cli/src/commands/order/calldata.rscrates/settings/src/local_db_remotes.rscrates/settings/ARCHITECTURE.md
📚 Learning: 2025-10-06T11:28:30.692Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2145
File: crates/common/src/raindex_client/local_db/query/fetch_orders/query.sql:6-7
Timestamp: 2025-10-06T11:28:30.692Z
Learning: In `crates/common/src/raindex_client/local_db/query/fetch_orders/query.sql`, the orderbook_address is currently hardcoded to '0x2f209e5b67A33B8fE96E28f24628dF6Da301c8eB' because the system only supports a single orderbook at the moment. Multiorderbook logic is not yet implemented and will be added in the future.
Applied to files:
crates/common/src/raindex_client/mod.rspackages/orderbook/test/common/test.test.tspackages/orderbook/test/js_api/raindexClient.test.tspackages/orderbook/test/js_api/gui.test.tscrates/settings/src/yaml/orderbook.rscrates/cli/src/commands/order/calldata.rscrates/settings/ARCHITECTURE.md
📚 Learning: 2025-06-10T12:04:54.107Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1916
File: packages/ui-components/src/lib/__fixtures__/settings-12-11-24.json:182-195
Timestamp: 2025-06-10T12:04:54.107Z
Learning: In test fixture files like `packages/ui-components/src/lib/__fixtures__/settings-12-11-24.json`, network configuration inconsistencies (such as matchain using Polygon's RPC, chainId, and currency while having its own network key) are acceptable since they are used for testing purposes only.
Applied to files:
crates/common/src/raindex_client/mod.rspackages/orderbook/test/js_api/raindexClient.test.ts
📚 Learning: 2025-10-29T13:29:29.578Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2278
File: packages/webapp/src/lib/services/startLocalDbSync.ts:23-29
Timestamp: 2025-10-29T13:29:29.578Z
Learning: In packages/webapp/src/lib/services/startLocalDbSync.ts, the hard-coded default orderbook address '0x2f209e5b67A33B8fE96E28f24628dF6Da301c8eB' is a temporary placeholder that will be removed when new sync logic is introduced in the Raindex client, as documented in the NOTE comment.
Applied to files:
crates/common/src/raindex_client/mod.rspackages/orderbook/test/common/test.test.tspackages/orderbook/test/js_api/raindexClient.test.tspackages/orderbook/test/js_api/gui.test.tscrates/settings/src/yaml/orderbook.rscrates/cli/src/commands/order/calldata.rscrates/settings/ARCHITECTURE.md
📚 Learning: 2025-04-08T12:53:12.526Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1588
File: packages/orderbook/test/js_api/gui.test.ts:2037-2057
Timestamp: 2025-04-08T12:53:12.526Z
Learning: In the remote network test for gui.test.ts, asserting that getCurrentDeployment() succeeds is sufficient validation that remote networks were properly fetched, as this operation would fail if the networks weren't correctly processed.
Applied to files:
crates/common/src/raindex_client/mod.rspackages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-10-06T08:05:02.438Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-06T08:05:02.438Z
Learning: Applies to packages/orderbook/**/*.{test,spec}.{ts,tsx} : Orderbook tests must pass (npm run test -w rainlanguage/orderbook)
Applied to files:
packages/orderbook/test/common/test.test.tspackages/orderbook/test/js_api/raindexClient.test.tspackages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-06-04T10:21:01.388Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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/common/test.test.tspackages/orderbook/test/js_api/raindexClient.test.tspackages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-07-04T09:14:35.748Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: packages/ui-components/src/__tests__/VaultDetail.test.ts:177-181
Timestamp: 2025-07-04T09:14:35.748Z
Learning: In the rain.orderbook project, maintainer findolor is fine with mutating mock data directly in test files using ts-expect-error comments, preferring this pragmatic approach over immutable patterns like spread operators for test data updates.
Applied to files:
packages/orderbook/test/common/test.test.tspackages/orderbook/test/js_api/raindexClient.test.tspackages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-08-15T20:56:15.592Z
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 1978
File: packages/webapp/src/routes/deploy/[orderName]/[deploymentKey]/fullDeployment.test.ts:22-29
Timestamp: 2025-08-15T20:56:15.592Z
Learning: In packages/webapp/src/routes/deploy/[orderName]/[deploymentKey]/fullDeployment.test.ts, brusherru prefers fail-fast testing approaches where type mismatches cause immediate test failures to alert developers about interface changes, rather than using defensive programming with type guards that might mask API evolution.
Applied to files:
packages/orderbook/test/common/test.test.tspackages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-07-01T21:26:42.195Z
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 1955
File: packages/orderbook/package.json:40-40
Timestamp: 2025-07-01T21:26:42.195Z
Learning: In the rain.orderbook project, the team does not require cross-platform compatibility improvements for the TypeScript check script in packages/orderbook/package.json. The current brace expansion syntax "./dist/**/*.{ts,js}" is sufficient for their development environment.
Applied to files:
packages/orderbook/test/common/test.test.tspackages/orderbook/test/js_api/raindexClient.test.tspackages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-03-26T16:16:51.943Z
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1515
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/layout.test.ts:37-37
Timestamp: 2025-03-26T16:16:51.943Z
Learning: For Rain Orderbook projects, in test files, the preference is to use "as any" type assertions with per-line ESLint disable comments rather than creating dedicated types for test parameters.
Applied to files:
packages/orderbook/test/common/test.test.ts
📚 Learning: 2025-10-06T08:05:02.438Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-06T08:05:02.438Z
Learning: Applies to packages/orderbook/**/*.{ts,tsx} : Orderbook TypeScript package must pass type/lint checks (npm run check -w rainlanguage/orderbook)
Applied to files:
packages/orderbook/test/common/test.test.ts
📚 Learning: 2025-04-04T11:25:21.518Z
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1559
File: packages/ui-components/src/__tests__/OrderOrVaultHash.test.ts:94-94
Timestamp: 2025-04-04T11:25:21.518Z
Learning: In the rain.orderbook project, minimal test fixtures are preferred over complete mocks that implement the entire interface. Type casting (e.g., `as unknown as SgVault`) is an acceptable approach to maintain both minimal fixtures and TypeScript type compatibility.
Applied to files:
packages/orderbook/test/common/test.test.tspackages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-07-09T12:35:45.699Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook 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.
Applied to files:
packages/orderbook/test/common/test.test.tspackages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-06-08T18:43:51.842Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1891
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/page.test.ts:66-80
Timestamp: 2025-06-08T18:43:51.842Z
Learning: In the rain.orderbook webapp test files, when mocking objects like the transaction manager, it's acceptable to use simple empty objects with ts-expect-error comments rather than providing complete mock implementations with all properties and methods.
Applied to files:
packages/orderbook/test/common/test.test.ts
📚 Learning: 2025-03-26T16:22:50.224Z
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1515
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/layout.test.ts:0-0
Timestamp: 2025-03-26T16:22:50.224Z
Learning: In the Rain Orderbook project, the DotrainOrderGui.getDeploymentDetail method resolves promises with objects that may contain either a value property or an error property, rather than rejecting promises on errors.
Applied to files:
packages/orderbook/test/common/test.test.ts
📚 Learning: 2025-04-02T08:11:19.742Z
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1566
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/+page.svelte:26-31
Timestamp: 2025-04-02T08:11:19.742Z
Learning: In the Rain Orderbook deployment page component, the conditional check `if (dotrain && deployment)` inside `onMount` is necessary to prevent GUI initialization with null values, even though there's already a redirect logic outside the function and conditional rendering for the UI message. These serve different purposes in the component's error handling strategy.
Applied to files:
packages/orderbook/test/common/test.test.ts
📚 Learning: 2025-06-11T11:39:15.239Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1917
File: tauri-app/src/routes/orders/add/+page.svelte:45-47
Timestamp: 2025-06-11T11:39:15.239Z
Learning: In the rain.orderbook codebase, every instance of `Config` (returned by helpers such as `parseDotrainAndYaml`) is guaranteed to include the `dotrainOrder` property; it is never `undefined`.
Applied to files:
packages/orderbook/test/common/test.test.ts
📚 Learning: 2025-07-17T10:35:09.329Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1996
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts:303-303
Timestamp: 2025-07-17T10:35:09.329Z
Learning: In packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts, findolor is fine with hardcoded 2-second delays using setTimeout in test cases for waiting after token selection, preferring this approach over deterministic waiting patterns.
Applied to files:
packages/orderbook/test/common/test.test.ts
📚 Learning: 2025-07-17T10:35:53.182Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1996
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts:172-172
Timestamp: 2025-07-17T10:35:53.182Z
Learning: In packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/fullDeployment.test.ts, findolor is comfortable with hardcoded 2-second delays using setTimeout after token selection elements appear, preferring this approach over deterministic waiting patterns with waitFor.
Applied to files:
packages/orderbook/test/common/test.test.ts
📚 Learning: 2025-07-04T10:23:41.820Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: packages/orderbook/test/js_api/raindexClient.test.ts:91-99
Timestamp: 2025-07-04T10:23:41.820Z
Learning: In the rain.orderbook project, maintainer findolor is fine with test code patterns that might seem like inverted logic in production contexts, such as the extractWasmEncodedData function in test files that returns undefined values when result.value is undefined.
Applied to files:
packages/orderbook/test/common/test.test.ts
📚 Learning: 2025-03-31T10:16:53.544Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1469
File: packages/ui-components/src/__tests__/CodeMirrorDotrain.test.ts:75-98
Timestamp: 2025-03-31T10:16:53.544Z
Learning: In the rainlanguage/rain.orderbook project, direct manipulation of Svelte's internal state (component.$$.ctx) in tests is an acceptable approach for testing component behavior, as demonstrated in the CodeMirrorDotrain tests.
Applied to files:
packages/orderbook/test/common/test.test.tspackages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-10-25T21:11:09.664Z
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 2246
File: packages/orderbook/scripts/sizeCheck.js:1-11
Timestamp: 2025-10-25T21:11:09.664Z
Learning: In the rain.orderbook repository, build scripts like sizeCheck.js intentionally lack defensive error handling (e.g., checking for missing arguments or file existence). The scripts should fail with raw errors to surface configuration or build pipeline issues that require immediate attention.
Applied to files:
packages/orderbook/test/common/test.test.ts
📚 Learning: 2025-06-07T05:19:04.767Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1872
File: packages/webapp/src/lib/services/handleVaultWithdraw.ts:50-54
Timestamp: 2025-06-07T05:19:04.767Z
Learning: In the rainlanguage/rain.orderbook codebase, the team prefers pragmatic code approaches over strict TypeScript patterns when the current implementation is sufficient for their use case, such as using `if (result.error)` instead of `if ('error' in result)` for discriminated union type checking.
Applied to files:
packages/orderbook/test/common/test.test.ts
📚 Learning: 2025-10-19T20:24:08.629Z
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 2246
File: packages/webapp/src/hooks.client.ts:2-2
Timestamp: 2025-10-19T20:24:08.629Z
Learning: In the rain.orderbook repository, the import `rainlanguage/orderbook/esm` in packages/webapp/src/hooks.client.ts is intentional and should not be changed to the root package path, as it's specific to the webapp build configuration.
Applied to files:
packages/orderbook/test/js_api/raindexClient.test.ts
📚 Learning: 2025-08-02T02:34:32.237Z
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2036
File: crates/js_api/src/filters/raindex_filter_store.rs:446-533
Timestamp: 2025-08-02T02:34:32.237Z
Learning: In the rainlanguage/rain.orderbook project's RaindexFilterStore WASM tests (crates/js_api/src/filters/raindex_filter_store.rs), brusherru decided to focus on testing only methods without side effects (that don't use web_sys) due to difficulties with mocking localStorage and window APIs in the WASM test environment. This pragmatic approach tests pure logic separately from browser integration.
Applied to files:
packages/orderbook/test/js_api/raindexClient.test.tscrates/settings/src/yaml/orderbook.rs
📚 Learning: 2025-04-09T13:00:24.640Z
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook 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.
Applied to files:
packages/orderbook/test/js_api/raindexClient.test.ts
📚 Learning: 2025-09-02T08:04:44.814Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2117
File: packages/ui-components/src/__tests__/VaultIdInformation.test.ts:9-13
Timestamp: 2025-09-02T08:04:44.814Z
Learning: In packages/ui-components/src/__tests__/VaultIdInformation.test.ts and similar test files in the rain.orderbook project, the passthrough vi.mock('rainlanguage/orderbook', async (importOriginal) => { return { ...(await importOriginal()) }; }); block is required for tests to run properly, even when not overriding any exports. This is needed due to the specific Vitest configuration or test environment setup in the project.
Applied to files:
packages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-07-09T14:00:12.206Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook 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.
Applied to files:
packages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-07-09T14:00:09.618Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1975
File: packages/orderbook/test/js_api/gui.test.ts:958-960
Timestamp: 2025-07-09T14:00:09.618Z
Learning: In test files for the rain.orderbook project, maintainer findolor prefers to keep hard-coded serialized state strings (like gzip/base64 blobs) rather than replacing them with semantic assertions or round-trip testing, even when the hard-coded approach may be more brittle to changes.
Applied to files:
packages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-07-11T08:46:07.606Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1925
File: packages/ui-components/src/__tests__/DeploymentSteps.test.ts:519-554
Timestamp: 2025-07-11T08:46:07.606Z
Learning: In packages/ui-components/src/__tests__/DeploymentSteps.test.ts, findolor prefers to keep hardcoded timeout values (like 50ms and 100ms) inline in test cases rather than extracting them to named constants, viewing such refactoring as unnecessary for test maintainability.
Applied to files:
packages/orderbook/test/js_api/gui.test.ts
📚 Learning: 2025-03-31T13:57:59.660Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1474
File: crates/js_api/src/yaml/mod.rs:31-34
Timestamp: 2025-03-31T13:57:59.660Z
Learning: The OrderbookYaml constructor in crates/js_api/src/yaml/mod.rs does not need early YAML validation. The author prefers to validate YAML only when it's actually used rather than during initialization.
Applied to files:
crates/settings/src/yaml/orderbook.rscrates/settings/ARCHITECTURE.md
📚 Learning: 2025-03-31T14:36:11.049Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1474
File: crates/js_api/src/yaml/mod.rs:37-44
Timestamp: 2025-03-31T14:36:11.049Z
Learning: The OrderbookYaml implementation in crates/js_api/src/yaml/mod.rs intentionally parses YAML on demand without caching results. This is a deliberate design choice by the author to process YAML only when needed rather than optimizing for repeated calls.
Applied to files:
crates/settings/src/yaml/orderbook.rscrates/settings/ARCHITECTURE.md
📚 Learning: 2025-04-07T09:54:21.782Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1483
File: crates/settings/src/yaml/orderbook.rs:45-45
Timestamp: 2025-04-07T09:54:21.782Z
Learning: The validation in OrderbookYaml's new method that includes RemoteNetworksCfg::parse_all_from_yaml is intentional and should remain as is, without conditional handling for users that only have local networks.
Applied to files:
crates/settings/src/yaml/orderbook.rscrates/settings/ARCHITECTURE.md
📚 Learning: 2025-09-01T13:35:33.688Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2116
File: crates/settings/src/orderbook.rs:569-0
Timestamp: 2025-09-01T13:35:33.688Z
Learning: In crates/settings/src/orderbook.rs, the deployment-block field is required in orderbook YAML configuration and should cause a parsing error when missing, not default to any value.
Applied to files:
crates/settings/src/yaml/orderbook.rscrates/cli/src/commands/order/calldata.rscrates/settings/src/local_db_remotes.rscrates/settings/ARCHITECTURE.md
📚 Learning: 2025-06-18T18:24:32.049Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook 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.
Applied to files:
crates/settings/src/yaml/orderbook.rscrates/settings/ARCHITECTURE.md
📚 Learning: 2025-07-11T06:40:49.511Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook 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.
Applied to files:
crates/settings/src/yaml/orderbook.rscrates/settings/src/local_db_remotes.rscrates/settings/ARCHITECTURE.md
📚 Learning: 2025-05-20T10:20:08.206Z
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook 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.
Applied to files:
crates/settings/src/yaml/orderbook.rs
📚 Learning: 2025-07-18T10:31:05.498Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook 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.
Applied to files:
crates/settings/src/yaml/orderbook.rs
📚 Learning: 2025-06-18T19:23:33.747Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1938
File: crates/settings/src/yaml/mod.rs:176-178
Timestamp: 2025-06-18T19:23:33.747Z
Learning: In crates/settings/src/yaml/mod.rs, the YamlError enum has two distinct error variants: `KeyNotFound(String)` for when a specific YAML key is not found in a hash/map, and `NotFound(String)` for when other types of entities (like networks, orderbooks, etc.) are not found in the configuration. These serve different purposes and should not be consolidated.
Applied to files:
crates/settings/src/yaml/orderbook.rscrates/settings/src/local_db_remotes.rs
📚 Learning: 2025-10-27T09:17:38.145Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2270
File: crates/settings/src/local_db_remotes.rs:107-212
Timestamp: 2025-10-27T09:17:38.145Z
Learning: In the rain.orderbook codebase using strict_yaml_rust, all YAML keys are treated as strings, including numeric literals. Numeric keys like `123` are automatically coerced to strings and are valid, so they don't need error-case testing for non-string keys.
Applied to files:
crates/settings/src/yaml/orderbook.rscrates/settings/src/local_db_remotes.rs
📚 Learning: 2025-09-24T10:54:42.060Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2163
File: crates/cli/src/commands/order/calldata.rs:47-48
Timestamp: 2025-09-24T10:54:42.060Z
Learning: In crates/cli/src/commands/order/calldata.rs, the config_deployment variable is used both as a parameter to AddOrderArgs::new_from_deployment() and later to extract RPC information, requiring the clone() to maintain access after ownership transfer.
Applied to files:
crates/cli/src/commands/order/calldata.rs
📚 Learning: 2025-06-17T16:46:19.035Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1903
File: crates/cli/src/commands/order/calldata.rs:47-57
Timestamp: 2025-06-17T16:46:19.035Z
Learning: In the CLI command `crates/cli/src/commands/order/calldata.rs`, the user prefers to let lower-level errors from `try_into_call()` bubble up when the RPCs list is empty, rather than adding early validation checks with custom error messages.
Applied to files:
crates/cli/src/commands/order/calldata.rs
📚 Learning: 2025-05-14T05:52:04.270Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook 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.
Applied to files:
crates/cli/src/commands/order/calldata.rs
📚 Learning: 2025-04-07T09:55:44.798Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook 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.
Applied to files:
crates/settings/src/local_db_remotes.rscrates/settings/ARCHITECTURE.md
📚 Learning: 2025-05-13T20:06:22.602Z
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook 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.
Applied to files:
crates/settings/src/local_db_remotes.rs
📚 Learning: 2025-05-16T17:26:09.529Z
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook 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.
Applied to files:
crates/settings/src/local_db_remotes.rs
📚 Learning: 2025-05-27T10:19:38.011Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1897
File: crates/settings/src/yaml/orderbook.rs:311-311
Timestamp: 2025-05-27T10:19:38.011Z
Learning: The rainlanguage/rain.orderbook project uses strict YAML parsing (StrictYaml library), so concerns about inconsistent parsing between integer and string values in YAML (like `spec-version: 1` vs `spec-version: "1"`) are not relevant to this codebase.
Applied to files:
crates/settings/src/local_db_remotes.rs
📚 Learning: 2025-09-02T08:04:34.729Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2117
File: crates/js_api/src/gui/select_tokens.rs:421-424
Timestamp: 2025-09-02T08:04:34.729Z
Learning: User findolor in the rainlanguage/rain.orderbook codebase prefers not to change test code when tests are already passing, even if there appear to be type mismatches or potential issues. Working tests should be left alone unless there are actual errors.
Applied to files:
crates/settings/src/local_db_remotes.rs
📚 Learning: 2025-07-11T08:37:24.423Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1983
File: crates/settings/src/gui.rs:965-973
Timestamp: 2025-07-11T08:37:24.423Z
Learning: In the rainlanguage/rain.orderbook project, user findolor prefers to keep user-facing error messages generic and user-friendly rather than technically specific. For example, using "a valid number" instead of "a valid u8 (0-255)" in GUI validation error messages to maintain simplicity for end users.
Applied to files:
crates/settings/src/local_db_remotes.rs
📚 Learning: 2025-07-16T10:40:05.717Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook 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<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/settings/src/local_db_remotes.rs
📚 Learning: 2025-06-18T12:56:44.290Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1938
File: crates/js_api/src/raindex/mod.rs:102-118
Timestamp: 2025-06-18T12:56:44.290Z
Learning: In the rainlanguage/rain.orderbook codebase, it's acceptable to scaffold unused enum variants in initial implementation PRs when they will be implemented in future PRs, as confirmed by findolor.
Applied to files:
crates/settings/src/local_db_remotes.rs
📚 Learning: 2025-07-04T09:02:57.301Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 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:
crates/settings/src/local_db_remotes.rs
📚 Learning: 2025-07-21T16:34:31.193Z
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 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/settings/src/local_db_remotes.rscrates/settings/ARCHITECTURE.md
📚 Learning: 2025-04-07T09:50:54.715Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1483
File: crates/settings/src/yaml/cache.rs:23-25
Timestamp: 2025-04-07T09:50:54.715Z
Learning: The Cache implementation in crates/settings/src/yaml/cache.rs returns cloned HashMaps in getter methods (like get_remote_networks) rather than references. This design choice was explicitly accepted by the maintainer.
Applied to files:
crates/settings/src/local_db_remotes.rs
📚 Learning: 2025-09-01T13:26:29.762Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2116
File: crates/common/src/raindex_client/mod.rs:0-0
Timestamp: 2025-09-01T13:26:29.762Z
Learning: In the rainlanguage/rain.orderbook codebase, the deployment-block field in orderbook YAML configuration cannot be absent - it is a required field, not optional as initially described in some documentation.
Applied to files:
crates/settings/ARCHITECTURE.md
📚 Learning: 2025-10-13T12:57:05.900Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2198
File: crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs:7-8
Timestamp: 2025-10-13T12:57:05.900Z
Learning: In the rain.orderbook repository, using unqualified serde helper names (e.g., `#[serde(with = "serde_address")]`) that rely on glob imports is acceptable and does not need to be changed to fully qualified paths.
Applied to files:
crates/settings/ARCHITECTURE.md
📚 Learning: 2025-06-17T16:57:25.456Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1903
File: crates/settings/src/config.rs:287-288
Timestamp: 2025-06-17T16:57:25.456Z
Learning: In the rain.orderbook repository, validation for empty `rpcs` vectors in `NetworkConfigSource` is not required, as indicated by the maintainer findolor.
Applied to files:
crates/settings/ARCHITECTURE.md
🧬 Code graph analysis (2)
crates/settings/src/yaml/orderbook.rs (3)
crates/settings/src/yaml/mod.rs (7)
should_validate_local_db_remotes(31-31)parse_all_from_yaml(65-68)parse_all_from_yaml(83-83)parse_from_yaml(70-79)parse_from_yaml(87-87)parse_from_yaml(95-98)new(42-42)crates/settings/src/local_db_remotes.rs (2)
parse_all_from_yaml(35-97)default(101-107)crates/settings/src/orderbook.rs (2)
parse_all_from_yaml(83-209)default(213-224)
crates/settings/src/local_db_remotes.rs (2)
crates/settings/src/yaml/mod.rs (4)
default_document(483-485)require_hash(410-439)require_string(370-405)get_document(491-494)crates/settings/src/orderbook.rs (4)
value(46-47)parse_all_from_yaml(83-209)default(213-224)eq(227-235)
🔇 Additional comments (8)
crates/cli/src/commands/order/calldata.rs (1)
197-199: YAML fixtures updated for local DB remotes — looks good.Top-level local-db-remotes entries are correctly added in test dotrain YAML. No code changes required here.
Also applies to: 479-481
packages/orderbook/test/js_api/gui.test.ts (3)
159-161: Add top-level local-db-remotes in GUI test fixtures — OK.Schema-aligned additions; no impact on existing assertions.
Also applies to: 241-243, 316-318
404-406: Per-orderbook local-db-remote linkage — OK.Explicit local-db-remote: remote references are consistent with the new top-level map.
Please confirm there’s at least one test asserting resolution failure when local-db-remote references a missing key (negative path), or point to coverage in settings/js_api tests.
Also applies to: 411-411, 417-418
984-984: Serialized state constant changed — verify regeneration.Confirm this gzip/base64 blob was regenerated from the updated codepaths (not hand‑edited) to prevent silent drift.
crates/settings/ARCHITECTURE.md (1)
123-128: OrderbookCfg docs updated with local_db_remote — aligned with impl.Accurately documents the new field and validators.
packages/orderbook/test/common/test.test.ts (2)
24-26: Add local-db-remotes in common test fixture — OK.Matches schema; no behavior change.
86-88: Stabilize expected Rainlang with join — good improvement.Less brittle against accidental whitespace/newline changes.
Also applies to: 107-109
packages/orderbook/test/js_api/raindexClient.test.ts (1)
48-50: Raindex client fixtures wired to local DB remotes — OK.Top-level map and per-orderbook references are consistent.
Also applies to: 56-56, 63-63
Comment on lines
+129
to
+134
| ### Local DB Remotes (`local_db_remotes.rs`) | ||
|
|
||
| - `local-db-remotes:` is a required top-level map. Each entry is parsed as `LocalDbRemoteCfg { key, url }`. | ||
| - The `orderbooks[*].local-db-remote` field is optional. If omitted, it defaults to the orderbook's key. When provided explicitly, it must reference a defined remote key under `local-db-remotes`. | ||
| - See `src/orderbook.rs` for the implementation and tests, e.g. `test_orderbook_local_db_remote_absent_defaults_to_orderbook_key`, `test_orderbook_local_db_remote_resolves`, and `test_orderbook_local_db_remote_not_found`. | ||
|
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Doc/spec alignment for defaults — nice; add YAML shape note.
You documented the optional default behavior. Also add these to “YAML Shape Reference” for completeness.
Apply this doc tweak:
@@ YAML Shape Reference (informative)
- - `orderbooks: { key: { address, network?, subgraph?, label?, deployment-block } }`
+ - `orderbooks: { key: { address, network?, subgraph?, label?, local-db-remote?, deployment-block } }`
+ - `local-db-remotes: { key: url }`Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In crates/settings/ARCHITECTURE.md around lines 129–134, add a short YAML Shape
Reference entry that documents the exact schema: state that top-level
local-db-remotes is a required map of remote-key → { url: string } (the key
itself is the map key, not duplicated), and that orderbooks[*].local-db-remote
is an optional string which, if omitted, defaults to the orderbook's key and if
present must match one of the keys in local-db-remotes; include a brief bulleted
shape example and a note referencing the existing tests
(`test_orderbook_local_db_remote_absent_defaults_to_orderbook_key`,
`test_orderbook_local_db_remote_resolves`,
`test_orderbook_local_db_remote_not_found`) for validation.
hardyjosh
approved these changes
Oct 31, 2025
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)
packages/webapp/src/lib/constants.ts(1 hunks)tauri-app/src/lib/services/loadRemoteSettings.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
tauri-app/**
📄 CodeRabbit inference engine (AGENTS.md)
Desktop app code is in tauri-app (Rust + Svelte); src-tauri is not part of the Cargo workspace
Files:
tauri-app/src/lib/services/loadRemoteSettings.ts
packages/webapp/**/*.{svelte,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Webapp must pass Svelte lint/format checks (npm run svelte-lint-format-check -w @rainlanguage/webapp)
Files:
packages/webapp/src/lib/constants.ts
packages/{webapp,ui-components}/**/*.{svelte,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
If you modify frontend code, you must include a screenshot of the built webapp reflecting the change
Files:
packages/webapp/src/lib/constants.ts
packages/**
📄 CodeRabbit inference engine (AGENTS.md)
JavaScript/Svelte projects live under packages/* (webapp, ui-components, orderbook)
Files:
packages/webapp/src/lib/constants.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 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: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 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
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the format: "TOTAL=number ADDITIONS=number DELETIONS=number". This report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 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).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the exact format: "TOTAL=number\nADDITIONS=number\nDELETIONS=number". This report should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: findolor
Repo: rainlanguage/rain.orderbook 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.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2145
File: crates/common/src/raindex_client/local_db/query/create_tables/query.sql:71-72
Timestamp: 2025-10-06T11:44:07.888Z
Learning: The local DB feature in the rain.orderbook codebase is not live yet (as of PR #2145), so schema migrations for existing databases are not required when modifying table structures in `crates/common/src/raindex_client/local_db/query/create_tables/query.sql`.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1483
File: crates/settings/src/yaml/orderbook.rs:45-45
Timestamp: 2025-04-07T09:54:21.782Z
Learning: The validation in OrderbookYaml's new method that includes RemoteNetworksCfg::parse_all_from_yaml is intentional and should remain as is, without conditional handling for users that only have local networks.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2202
File: crates/common/src/raindex_client/local_db/sync.rs:33-34
Timestamp: 2025-10-14T07:51:55.148Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the hard-coded `DEFAULT_SYNC_CHAIN_ID` constant (set to `SUPPORTED_LOCAL_DB_CHAINS[0]`) will be replaced with proper chain ID handling in downstream PRs as part of the multi-network/orderbook implementation.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2237
File: crates/common/src/raindex_client/local_db/sync.rs:79-89
Timestamp: 2025-10-18T10:38:41.273Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the sync_database method currently only supports indexing a single orderbook per chain ID, which is why `.first()` is used to select the orderbook configuration. Multi-orderbook support per chain ID is planned for future PRs.
📚 Learning: 2025-10-19T20:24:08.629Z
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 2246
File: packages/webapp/src/hooks.client.ts:2-2
Timestamp: 2025-10-19T20:24:08.629Z
Learning: In the rain.orderbook repository, the import `rainlanguage/orderbook/esm` in packages/webapp/src/hooks.client.ts is intentional and should not be changed to the root package path, as it's specific to the webapp build configuration.
Applied to files:
packages/webapp/src/lib/constants.ts
📚 Learning: 2025-04-09T12:58:03.399Z
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1565
File: packages/webapp/src/lib/services/loadRegistryUrl.ts:5-19
Timestamp: 2025-04-09T12:58:03.399Z
Learning: In the rain.orderbook application, URL validation for registry URLs is handled at the frontend/component level before the `loadRegistryUrl` function is called, so additional validation within this function is not necessary.
Applied to files:
packages/webapp/src/lib/constants.ts
📚 Learning: 2025-04-09T13:00:24.640Z
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook 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.
Applied to files:
packages/webapp/src/lib/constants.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). (16)
- GitHub Check: test
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- 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-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- 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-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: test
- GitHub Check: git-clean
- GitHub Check: test
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: Deploy-Preview-Push
🔇 Additional comments (2)
packages/webapp/src/lib/constants.ts (1)
1-4: No issues found—both URLs are accessible with valid content.Both
REGISTRY_URLandREMOTE_SETTINGS_URLreturn HTTP 200 and contain valid, substantive content. The registry file lists strategies, and the settings.yaml file contains proper network configuration. The commit hash reference is correct and accessible.tauri-app/src/lib/services/loadRemoteSettings.ts (1)
2-2: No issues found—the remote URL is accessible and valid.The verification confirms that the GitHub URL points to a valid, accessible resource with HTTP Status 200 and returns properly formatted YAML content at the specified commit hash.
| @@ -1,5 +1,5 @@ | |||
| const REMOTE_SETTINGS_URL = | |||
| 'https://raw.githubusercontent.com/rainlanguage/rain.strategies/d57c3aa77b63f90535957947d15146c0182367dc/settings.yaml'; | |||
| 'https://raw.githubusercontent.com/rainlanguage/rain.strategies/c7f57848b8974c3190027749bf4406d2551bed9f/settings.yaml'; | |||
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider consolidating duplicate URL constants.
REMOTE_SETTINGS_URL is defined in both this file and packages/webapp/src/lib/constants.ts with identical values. This creates a maintenance burden and risk of drift if they're updated independently.
Consider:
- Exporting the constant from a single shared location
- Importing it in both files
- Or documenting why separate definitions are necessary if they serve different purposes
Example approach - create a shared constants file:
// shared/constants.ts
export const REMOTE_SETTINGS_URL =
'https://raw.githubusercontent.com/rainlanguage/rain.strategies/c7f57848b8974c3190027749bf4406d2551bed9f/settings.yaml';Then import in both locations:
import { REMOTE_SETTINGS_URL } from '../shared/constants';🤖 Prompt for AI Agents
In tauri-app/src/lib/services/loadRemoteSettings.ts around line 2,
REMOTE_SETTINGS_URL is duplicated across this file and
packages/webapp/src/lib/constants.ts; consolidate by extracting the constant
into a single shared module (e.g., shared/constants or a central package-level
constants file), export REMOTE_SETTINGS_URL from there, update both files to
import that exported constant instead of redefining it, remove the duplicate
definition from this file, and run the project build/tests to ensure imports
resolve correctly (or document and leave as-is only if there is a deliberate,
justified divergence).
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 |
This was referenced Oct 31, 2025
Merged
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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
See issues:
See PRs:
Solution
We can now remote urls (manifest yaml) per orderbook cfg:
local-db-remotestop level orderbook yaml field.local-db-remotefield to orderbook configuration.local-db-remotesfield.Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Tests
Refactor
Chores