Remove post batch functionality and register views during bootstrap#2325
Merged
Remove post batch functionality and register views during bootstrap#2325
Conversation
Contributor
WalkthroughRemoves the ApplyPipeline post-batch hook and consolidates vault-balance upserts into the main batch; adds a new create_views batch (vault_deltas view) and executes it during bootstrap; replaces ClientApplyAdapter usages with DefaultApplyPipeline and updates tests to assert view-creation statements. Changes
Sequence Diagram(s)sequenceDiagram
participant Engine
participant ApplyPipeline
participant Persist
rect rgb(220, 240, 255)
Note over Engine,ApplyPipeline: Old flow (with post-batch)
Engine->>ApplyPipeline: build_batch(target_info)
ApplyPipeline-->>Engine: batch_main
Engine->>ApplyPipeline: build_post_batch(target_info)
ApplyPipeline-->>Engine: batch_post
Engine->>Engine: extend batch_main with batch_post
Engine->>Persist: execute combined batch
end
rect rgb(240, 220, 255)
Note over Engine,ApplyPipeline: New flow (consolidated)
Engine->>ApplyPipeline: build_batch(target_info)
ApplyPipeline-->>Engine: batch_all (tables, metadata, vault balances)
Engine->>Persist: execute batch_all
end
sequenceDiagram
participant Bootstrap
participant DB
participant CreateViews
rect rgb(220, 240, 255)
Note over Bootstrap,DB: Bootstrap sequence with view creation
Bootstrap->>DB: clear_tables()
Bootstrap->>DB: create_tables()
Bootstrap->>DB: insert_db_metadata()
Bootstrap->>CreateViews: create_views_batch()
CreateViews-->>Bootstrap: view_batch (BEGIN, vault_deltas SQL, COMMIT)
Bootstrap->>DB: execute_batch(view_batch)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
🧰 Additional context used📓 Path-based instructions (3)crates/**/*.rs📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/crates/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.rs📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (17)📓 Common learnings📚 Learning: 2025-07-16T05:52:05.576ZApplied to files:
📚 Learning: 2025-10-06T14:39:36.425ZApplied to files:
📚 Learning: 2025-05-21T22:14:22.646ZApplied to files:
📚 Learning: 2025-05-21T21:26:31.566ZApplied to files:
📚 Learning: 2025-05-21T22:14:22.646ZApplied to files:
📚 Learning: 2025-05-21T21:24:53.708ZApplied to files:
📚 Learning: 2025-07-09T07:42:48.458ZApplied to files:
📚 Learning: 2025-05-21T21:25:47.873ZApplied to files:
📚 Learning: 2025-05-21T21:25:47.873ZApplied to files:
📚 Learning: 2025-05-21T22:46:08.530ZApplied to files:
📚 Learning: 2025-10-18T10:38:41.273ZApplied to files:
📚 Learning: 2025-10-06T11:28:30.692ZApplied to files:
📚 Learning: 2025-06-24T08:46:03.368ZApplied to files:
📚 Learning: 2025-05-27T06:16:12.195ZApplied to files:
📚 Learning: 2025-10-06T11:13:29.956ZApplied to files:
📚 Learning: 2025-10-14T07:51:55.148ZApplied to files:
🧬 Code graph analysis (1)crates/common/src/local_db/pipeline/adapters/apply.rs (1)
🔇 Additional comments (7)
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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
crates/cli/src/commands/local_db/pipeline/bootstrap.rs(6 hunks)crates/cli/src/commands/local_db/pipeline/runner/mod.rs(0 hunks)crates/common/src/local_db/pipeline/adapters/apply.rs(4 hunks)crates/common/src/local_db/pipeline/adapters/bootstrap.rs(7 hunks)crates/common/src/local_db/pipeline/engine.rs(1 hunks)crates/common/src/local_db/query/create_views/mod.rs(1 hunks)crates/common/src/local_db/query/mod.rs(1 hunks)crates/common/src/raindex_client/local_db/pipeline/apply.rs(0 hunks)crates/common/src/raindex_client/local_db/pipeline/bootstrap.rs(6 hunks)crates/common/src/raindex_client/local_db/pipeline/mod.rs(0 hunks)crates/common/src/raindex_client/local_db/pipeline/runner/environment.rs(3 hunks)crates/common/src/raindex_client/local_db/pipeline/runner/mod.rs(2 hunks)crates/common/src/raindex_client/local_db/pipeline/runner/scheduler.rs(2 hunks)
💤 Files with no reviewable changes (3)
- crates/cli/src/commands/local_db/pipeline/runner/mod.rs
- crates/common/src/raindex_client/local_db/pipeline/mod.rs
- crates/common/src/raindex_client/local_db/pipeline/apply.rs
🧰 Additional context used
🧠 Learnings (13)
📓 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: findolor
Repo: rainlanguage/rain.orderbook PR: 1994
File: crates/common/src/raindex_client/vaults.rs:59-59
Timestamp: 2025-07-16T05:52:05.576Z
Learning: User findolor prefers to handle documentation updates for getter methods in batch via dedicated PRs rather than addressing them individually during feature development, as mentioned for the formatted amount string fields in crates/common/src/raindex_client/vaults.rs.
Learnt from: 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: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: 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: 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).
📚 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/local_db/pipeline/runner/scheduler.rscrates/common/src/raindex_client/local_db/pipeline/runner/environment.rscrates/common/src/raindex_client/local_db/pipeline/bootstrap.rscrates/common/src/raindex_client/local_db/pipeline/runner/mod.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/local_db/pipeline/adapters/apply.rscrates/common/src/raindex_client/local_db/pipeline/bootstrap.rs
📚 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/local_db/pipeline/adapters/apply.rs
📚 Learning: 2025-06-24T08:46:03.368Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1947
File: crates/common/src/raindex_client/orders.rs:98-125
Timestamp: 2025-06-24T08:46:03.368Z
Learning: In the vault merging logic in crates/common/src/raindex_client/orders.rs, optimization isn't necessary because the maximum list items are usually around 5 items. For such small datasets, the simple three-loop approach is preferred over HashMap-based optimization due to clarity and minimal performance impact.
Applied to files:
crates/common/src/local_db/pipeline/adapters/apply.rs
📚 Learning: 2025-05-27T06:16:12.195Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1874
File: tauri-app/src-tauri/src/transaction_status.rs:87-114
Timestamp: 2025-05-27T06:16:12.195Z
Learning: In tauri-app/src-tauri/src/transaction_status.rs, testing one WriteTransactionStatus variant (like PendingPrepare) is sufficient for the update_status_and_emit method since all variants follow the same code path through the From<WriteTransactionStatus<T>> implementation.
Applied to files:
crates/common/src/local_db/pipeline/adapters/apply.rs
📚 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/local_db/pipeline/bootstrap.rs
📚 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/local_db/pipeline/bootstrap.rscrates/common/src/raindex_client/local_db/pipeline/runner/mod.rs
📚 Learning: 2025-10-06T14:41:41.909Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2159
File: crates/cli/src/commands/local_db/sync/runner/mod.rs:52-113
Timestamp: 2025-10-06T14:41:41.909Z
Learning: The local DB sync CLI command (crates/cli/src/commands/local_db/sync/) is designed for CI-only usage, and simple println! statements are preferred over structured logging for status messages.
Applied to files:
crates/common/src/raindex_client/local_db/pipeline/bootstrap.rscrates/common/src/local_db/pipeline/adapters/bootstrap.rscrates/cli/src/commands/local_db/pipeline/bootstrap.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/common/src/raindex_client/local_db/pipeline/bootstrap.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/common/src/raindex_client/local_db/pipeline/bootstrap.rs
📚 Learning: 2025-07-15T08:01:38.534Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1994
File: crates/common/src/raindex_client/vaults.rs:282-292
Timestamp: 2025-07-15T08:01:38.534Z
Learning: In the rainlanguage/rain.orderbook codebase, findolor prefers to avoid concurrency optimizations like using `futures::future::try_join_all` for parallel processing of balance changes, considering such optimizations "not that critical at the moment" when the performance impact is minimal.
Applied to files:
crates/common/src/raindex_client/local_db/pipeline/bootstrap.rs
📚 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/common/src/raindex_client/local_db/pipeline/bootstrap.rs
🔇 Additional comments (10)
crates/common/src/local_db/query/mod.rs (1)
1-7: Exposecreate_viewsmodule looks correctAdding
pub mod create_views;cleanly exposes the new view helpers alongside other query modules; matches downstream usage in bootstrap and tests.crates/cli/src/commands/local_db/pipeline/bootstrap.rs (1)
58-81: View wiring in CLI bootstrap tests is sound
MockDb::with_viewsseeding allcreate_views_batch()statements intotext_mapensuresexecute_batchduringreset_dbwon’t fail due to missing mappings, and the newengine_run_executes_view_creationtest correctly asserts that every view creation statement is executed duringengine_run. Updating the other tests to chain.with_views()keeps their expectations valid under the newreset_dbbehavior without changing their semantics.Also applies to: 113-146, 148-178, 180-190, 220-229, 260-266
crates/common/src/raindex_client/local_db/pipeline/runner/scheduler.rs (1)
3-6: Scheduler now targetsDefaultApplyPipelineappropriatelyUpdating the
DefaultClientRunneralias to useDefaultApplyPipeline(and importing it alongsideApplyPipeline) keeps the scheduler wiring aligned with the new default apply implementation while preserving the existingSchedulerRunnertrait implementation andstartAPI.Also applies to: 24-32
crates/common/src/local_db/pipeline/engine.rs (1)
301-321: Apply phase simplification to a single batch is consistent
apply_changesnow builds a single batch viaApplyPipeline::build_batchand persists it before exporting the dump, removing the separate post-batch step. This keeps the engine-side control flow simple while leaving room for concreteApplyPipelineimplementations (like the new default one) to encapsulate any extra work insidebuild_batch, and existing tests around build/persist/export error propagation remain valid.crates/common/src/local_db/pipeline/adapters/bootstrap.rs (1)
1-6: Registering view creation inreset_dbis correct and well-testedHooking
create_views_batch()intoreset_dbafter table creation and metadata insert ensures any freshly reset local DB always has the required views. TheMockDb::with_viewshelper plus the updatedreset_db_*tests (which deriveexpected_viewsfromcreate_views_batch()and assert both the statement count and ordering) give good coverage for both success and failure paths, including version overrides and error propagation before the view batch runs. Given the local DB feature isn’t live yet, baking view creation into the default reset path without a separate migration path is a safe choice. Based on learningsAlso applies to: 102-118, 163-190, 577-609, 611-632, 634-656, 658-674
crates/common/src/raindex_client/local_db/pipeline/runner/mod.rs (1)
5-23: Client runner now consistently usesDefaultApplyPipelineImporting
DefaultApplyPipelineand updating the concreteClientRunner<...>specialization fornewto use it brings the RAIndex client runner in line with the rest of the pipeline wiring (scheduler/environment), without impacting the test harness that relies on stubApplyPipelineimplementations.Also applies to: 153-168
crates/common/src/local_db/query/create_views/mod.rs (1)
1-35: View batch helper is minimal, transactional, and well covered by testsThe API cleanly exposes a single‑view batch with
ensure_transaction(), and the tests validate both transactional wrapping and SQL equality/params, which should keep this in sync with the underlyingvault_deltas.sql.crates/common/src/raindex_client/local_db/pipeline/runner/environment.rs (1)
1-39: Switch toDefaultApplyPipelineis consistent with existing runner wiringThe updated generic parameter and constructor argument align with the adapters ordering in
RunnerEnvironment/EnginePipelines, so the client runner now uses the same default apply pipeline implementation as other environments without altering the existing test expectations.crates/common/src/raindex_client/local_db/pipeline/bootstrap.rs (2)
263-265: Reset-path tests thoroughly verify that view creation is part ofreset_dbThe three
runner_run_*reset*tests now all assert that, afterclear_tables/create_tables/insert_db_metadata, the expected view statements fromcreate_views_batchare executed (in order for the first test, and by presence for the others). This gives good coverage that schema resets consistently include view creation across all mismatch/missing-metadata cases.Also applies to: 271-276, 283-283, 305-307, 313-317, 321-324, 353-355, 362-367, 369-372
439-447: Fresh-db dump path remains minimal and side-effect freeThe
engine_run_applies_dump_on_fresh_dbtest continues to assert that a fresh DB with a dump configured only executes the dump statement and nothing else, which is important now that additional bootstrap-time work (like views) happens exclusively via the runner path.
Comment on lines
+347
to
357
| // Expect vault balance refresh plus the watermark when there is no work. | ||
| let texts: Vec<_> = batch.statements().iter().map(|s| s.sql().trim()).collect(); | ||
| assert_eq!(texts.len(), 1); | ||
| assert_eq!(texts.len(), 3); | ||
| assert!(texts.iter().any(|s| s.contains("vault_balance_changes"))); | ||
| assert!(texts | ||
| .iter() | ||
| .any(|s| s.contains("INSERT OR REPLACE INTO running_vault_balances"))); | ||
| assert!(texts | ||
| .iter() | ||
| .all(|s| s.contains("INSERT INTO target_watermarks"))); | ||
| .any(|s| s.contains("INSERT INTO target_watermarks"))); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Empty-work-window test now correctly asserts vault balance refresh + watermark
The updated expectations (vault_balance_changes, running_vault_balances, and a single target_watermarks insert) align with the new batch contents for a no‑work window. The function name still says “only watermark”, so renaming it to mention vault balances would better reflect the current behavior.
🤖 Prompt for AI Agents
crates/common/src/local_db/pipeline/adapters/apply.rs around lines 347 to 357:
the test function name still says it tests “only watermark” but the assertions
expect a vault balance refresh plus watermark; rename the test to reflect vault
balances (e.g., include “vault_balance” or “vault_balance_and_watermark”),
update any doc comment or test description string to match, and search/replace
any references to the old test name (including mod declarations or invoke
points) so the name change stays consistent.
Contributor
There was a problem hiding this comment.
@findolor, understood. I'll leave the test name as is.
🧠 Learnings used
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1874
File: tauri-app/src-tauri/src/transaction_status.rs:87-114
Timestamp: 2025-05-27T06:16:12.195Z
Learning: In tauri-app/src-tauri/src/transaction_status.rs, testing one WriteTransactionStatus variant (like PendingPrepare) is sufficient for the update_status_and_emit method since all variants follow the same code path through the From<WriteTransactionStatus<T>> implementation.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2277
File: crates/common/src/local_db/query/create_tables/query.sql:11-19
Timestamp: 2025-10-28T14:11:56.648Z
Learning: In the target_watermarks table (crates/common/src/local_db/query/create_tables/query.sql), additional indexes beyond the composite primary key (chain_id, orderbook_address) are not needed because the table will have a small number of rows.
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2036
File: packages/orderbook/test/js_api/filters.test.ts:19-36
Timestamp: 2025-08-02T03:55:25.215Z
Learning: In the rainlanguage/rain.orderbook project's WASM tests, the pattern of chaining `.value!` calls on WASM result types (like from VaultsFilterBuilder methods) is the established and preferred approach for handling WASM results, and should not be refactored into intermediate variables as it would add unnecessary verbosity without improving the code.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1994
File: crates/common/src/raindex_client/vaults.rs:59-59
Timestamp: 2025-07-16T05:52:05.576Z
Learning: User findolor prefers to handle documentation updates for getter methods in batch via dedicated PRs rather than addressing them individually during feature development, as mentioned for the formatted amount string fields in crates/common/src/raindex_client/vaults.rs.
Learnt from: 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: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: 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: 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).
| use crate::local_db::query::clear_tables::clear_tables_stmt; | ||
| use crate::local_db::query::create_tables::create_tables_stmt; | ||
| use crate::local_db::query::create_tables::REQUIRED_TABLES; | ||
| use crate::local_db::query::create_views::create_views_batch; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
with_views helper cleanly mirrors the real view batch in tests
Importing create_views_batch and deriving all mocked responses from its statements ensures the test DB stays aligned with the actual view-creation SQL (including transaction wrapper statements) without duplicating strings.
If additional view batches are added in the future, you might consider composing them inside with_views() as well so tests automatically cover the full reset behavior.
Also applies to: 165-170
🤖 Prompt for AI Agents
crates/common/src/raindex_client/local_db/pipeline/bootstrap.rs around lines 124
(and likewise 165-170): the with_views test helper duplicates SQL for view
creation which can drift from real code; import and call create_views_batch from
crate::local_db::query::create_views to obtain the authoritative statements
(including transaction wrapper statements) and derive the mocked responses from
that returned statements list instead of hardcoding strings; update with_views
to iterate the statements from create_views_batch and produce the same mock
results/order as the real pipeline, and apply the same refactor to the helper
usage around lines 165-170 so tests mirror any future added view batches
automatically.
Contributor
There was a problem hiding this comment.
@findolor, you're absolutely right! Looking at the code, with_views already uses create_views_batch() to derive all mocked responses from the actual statements, which keeps the tests aligned with the real view-creation SQL. The implementation is already following the suggested pattern. No changes needed here.
🧠 Learnings used
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: 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.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2159
File: crates/cli/src/commands/local_db/sync/runner/mod.rs:52-113
Timestamp: 2025-10-06T14:41:41.909Z
Learning: The local DB sync CLI command (crates/cli/src/commands/local_db/sync/) is designed for CI-only usage, and simple println! statements are preferred over structured logging for status messages.
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: 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.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1994
File: crates/common/src/raindex_client/vaults.rs:59-59
Timestamp: 2025-07-16T05:52:05.576Z
Learning: User findolor prefers to handle documentation updates for getter methods in batch via dedicated PRs rather than addressing them individually during feature development, as mentioned for the formatted amount string fields in crates/common/src/raindex_client/vaults.rs.
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1790
File: tauri-app/src-tauri/src/commands/vault.rs:67-67
Timestamp: 2025-05-17T15:32:28.733Z
Learning: For the PR focused on testing Tauri commands::order module, the generic type parameter R: Runtime was selectively added where needed for the PR scope, applying the changes primarily to order.rs and related files while leaving other modules like vault.rs for potential future refactoring.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1947
File: crates/common/src/raindex_client/orders.rs:98-125
Timestamp: 2025-06-24T08:46:03.368Z
Learning: In the vault merging logic in crates/common/src/raindex_client/orders.rs, optimization isn't necessary because the maximum list items are usually around 5 items. For such small datasets, the simple three-loop approach is preferred over HashMap-based optimization due to clarity and minimal performance impact.
Learnt from: 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: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 format: "TOTAL=number ADDITIONS=number DELETIONS=number". This report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).
0xgleb
approved these changes
Nov 25, 2025
hardyjosh
approved these changes
Nov 30, 2025
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. 🧩 Analysis chain🏁 Script executed: Length of output: 51 SIZE=L 🧠 Learnings used |
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.
Chained PR
Motivation
Now that we have the float related database functions, we can process vault balances on the producer runner. That means we don't need the post batch functionality that was only used by the client runner. The same calculations can be made on the producer and included in the default build batch logic for apply pipeline.
With that we needed the vault deltas view on the producer so it must included in the bootstrapping phase.
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.