Add granular per-network sync status UI with phase details and error reporting #2381
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…it to accept typed phases instead of strings
…ey from NetworkSyncStatus, and add serialization/deserialization tests
…ase emission, orderbook-scoped status updates, and unit/wasm tests
…er state, and key network status by chainId
…ry all when chain_ids is None/empty, remove obsolete order routing test, and add vault query tests
…ified updateStatus callback, and tests for store functions and callback integration
…book sync status, leader-aware phase display (hidden when notLeader), and comprehensive tests
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe PR refactors the local DB sync status system from string-based messaging to a type-safe Changes
Sequence DiagramsequenceDiagram
actor User
participant Engine as Sync Engine
participant StatusBus as ClientStatusBus
participant Callback as JS Callback
participant Store as localDbStatus Store
participant UI as UI Components
Engine->>Engine: Emit SyncPhase::<br/>FetchingLatestBlock
Engine->>StatusBus: send(phase)
StatusBus->>StatusBus: Check leader state &<br/>ob_id present
alt Leader & has ob_id
StatusBus->>StatusBus: Convert to<br/>OrderbookSyncStatus::syncing
StatusBus->>Callback: invoke callback(status)
end
Callback->>Store: updateStatus(status)
Store->>Store: Route to<br/>updateOrderbookStatus
Store->>Store: Update orderbookStatuses map
Store->>Store: Compute aggregateStatus<br/>(failure/syncing logic)
Store-->>UI: Reactive update
Engine->>Engine: Emit SyncPhase::<br/>DecodingOrderbookLogs
Engine->>StatusBus: send(phase)
StatusBus->>Callback: invoke callback(phase_updated)
Callback->>Store: updateStatus(...)
Store-->>UI: Modal shows<br/>current phase
User->>UI: Click network<br/>status header
UI->>UI: Open LocalDbStatusModal
UI->>UI: Render per-network<br/>groups with phase
UI-->>User: Display sync progress
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
Comment |
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Comment on lines
+30
to
+38
| fn emit_to_callback(status: OrderbookSyncStatus) { | ||
| STATUS_CALLBACK.with(|c| { | ||
| if let Some(callback) = c.borrow().as_ref() { | ||
| if let Ok(value) = serde_wasm_bindgen::to_value(&status) { | ||
| let _ = callback.call1(&JsValue::NULL, &value); | ||
| } | ||
| } | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Silent failure on serialization error could mask issues.
The emit_to_callback function silently ignores serialization failures via the if let Ok(value) pattern. While this prevents crashes, it could mask issues during development. Consider logging a warning (via web_sys::console::warn_1) when serialization fails, at least in debug builds.
🔎 Optional: Add debug logging for serialization failures
fn emit_to_callback(status: OrderbookSyncStatus) {
STATUS_CALLBACK.with(|c| {
if let Some(callback) = c.borrow().as_ref() {
- if let Ok(value) = serde_wasm_bindgen::to_value(&status) {
- let _ = callback.call1(&JsValue::NULL, &value);
+ match serde_wasm_bindgen::to_value(&status) {
+ Ok(value) => {
+ let _ = callback.call1(&JsValue::NULL, &value);
+ }
+ Err(_e) => {
+ #[cfg(debug_assertions)]
+ web_sys::console::warn_1(&"Failed to serialize status for callback".into());
+ }
}
}
});
}🤖 Prompt for AI Agents
In crates/common/src/raindex_client/local_db/pipeline/status.rs around lines 30
to 38, the function currently swallows serde_wasm_bindgen serialization errors;
change it so that when to_value fails you emit a warning containing the error
details (use web_sys::console::warn_1) so failures are visible during
development — perform the warning only in debug builds (cfg!(debug_assertions)
or conditional compilation) and convert the error message to a JsValue (e.g.
JsValue::from_str(&format!("{:?}", err))) before passing to console::warn_1;
keep the existing successful-path behavior unchanged.
Collaborator
Author
There was a problem hiding this comment.
we know that we are not going to have any issues so no need to log something
Contributor
There was a problem hiding this comment.
@findolor, understood! I'll skip the logging suggestion. The serialization path is straightforward with these well-defined types, so the extra defensive logging isn't necessary here.
🧠 Learnings used
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.
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: 2344
File: crates/common/src/local_db/pipeline/runner/mod.rs:18-31
Timestamp: 2025-12-03T10:40:25.429Z
Learning: In `crates/common/src/local_db/pipeline/runner/mod.rs`, the `TargetSuccess` struct does not need separate `ob_id` or `orderbook_key` fields because the contained `SyncOutcome` already includes orderbook identification information such as chain_id and orderbook_address. This avoids redundant data duplication.
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: 2245
File: crates/common/src/raindex_client/local_db/executor.rs:17-46
Timestamp: 2025-10-19T15:52:58.239Z
Learning: When formatting JsValue errors in Rust wasm code using `{:?}`, the Debug output is sufficiently informative and does not produce unhelpful "[object Object]" strings in practice.
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2036
File: crates/js_api/src/filters/errors.rs:39-46
Timestamp: 2025-08-01T07:35:13.418Z
Learning: In the Rain Orderbook project's PersistentFilterStoreError (crates/js_api/src/filters/errors.rs), the Display implementation already provides user-friendly error messages, so using err.to_string() for both msg and readable_msg in the WasmEncodedError conversion is appropriate and doesn't require separate readable message handling.
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.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1715
File: crates/js_api/src/common/mod.rs:55-59
Timestamp: 2025-05-14T05:51:50.277Z
Learning: In the Rain Orderbook project, the error handling for WASM exported functions uses WasmEncodedError which includes both the original error message and a human-readable version, making direct modification of the JsValue conversion unnecessary.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1858
File: crates/subgraph/src/orderbook_client/mod.rs:54-58
Timestamp: 2025-05-19T13:40:56.080Z
Learning: The `wasm_bindgen_utils` crate in the Rain Orderbook project handles conditional compilation for `JsValue` and `JsError` internally, allowing `impl From<Error> for JsValue` to work on non-WASM targets without explicit cfg guards.
Learnt from: 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: 1903
File: crates/settings/src/yaml/orderbook.rs:371-377
Timestamp: 2025-06-17T16:21:24.384Z
Learning: In crates/settings/src/yaml/orderbook.rs tests, the user findolor considers RPC ordering in Vec<Url> assertions to be intentional and not a test brittleness issue. The ordering of RPCs in tests should be preserved as specified.
Learnt from: findolor
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.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 2246
File: packages/orderbook/scripts/buildEsm.js:38-53
Timestamp: 2025-10-25T21:07:19.704Z
Learning: In the rain.orderbook repository, the deduplication logic in buildEsm.js (and similar build scripts) intentionally lacks defensive checks for indexOf returning -1. If duplicates are not found, the script should throw to surface breaking changes in wasm-bindgen output that require attention.
Comment on lines
+11
to
+23
| export let networkStatuses: Map<number, NetworkSyncStatus> = new Map(); | ||
| export let orderbookStatuses: Map<string, OrderbookSyncStatus> = new Map(); | ||
| let copied = false; | ||
| let copyErrorFailed = false; | ||
| let copyResetTimeout: ReturnType<typeof setTimeout> | undefined; | ||
| let modalOpen = false; | ||
| onDestroy(() => { | ||
| if (copyResetTimeout) { | ||
| clearTimeout(copyResetTimeout); | ||
| } | ||
| }); | ||
| $: networkList = Array.from(networkStatuses.values()); | ||
| $: hasNetworks = networkList.length > 0; | ||
| $: hasFailure = networkList.some((s) => s.status === 'failure'); | ||
| $: displayStatus = (hasFailure ? 'failure' : 'active') as LocalDbStatus; | ||
| const copyError = async () => { | ||
| if (!error || typeof navigator === 'undefined' || !navigator.clipboard?.writeText) { | ||
| return; | ||
| } | ||
| if (copyResetTimeout) { | ||
| clearTimeout(copyResetTimeout); | ||
| copyResetTimeout = undefined; | ||
| } | ||
| copyErrorFailed = false; | ||
| try { | ||
| await navigator.clipboard.writeText(error); | ||
| copied = true; | ||
| copyResetTimeout = setTimeout(() => { | ||
| copied = false; | ||
| copyResetTimeout = undefined; | ||
| }, 2500); | ||
| } catch { | ||
| copied = false; | ||
| copyErrorFailed = true; | ||
| } | ||
| }; | ||
| function openModal() { | ||
| modalOpen = true; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider including 'syncing' in displayStatus derivation.
The displayStatus is derived as failure if any network has failure status, otherwise active. However, the syncing status is not considered, which means actively syncing networks will show as active in the badge.
Is this intentional? If the card should indicate ongoing sync activity, consider:
🔎 Optional: Include syncing status in displayStatus
$: hasFailure = networkList.some((s) => s.status === 'failure');
-$: displayStatus = (hasFailure ? 'failure' : 'active') as LocalDbStatus;
+$: hasSyncing = networkList.some((s) => s.status === 'syncing');
+$: displayStatus = (hasFailure ? 'failure' : hasSyncing ? 'syncing' : 'active') as LocalDbStatus;📝 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
| export let networkStatuses: Map<number, NetworkSyncStatus> = new Map(); | |
| export let orderbookStatuses: Map<string, OrderbookSyncStatus> = new Map(); | |
| let copied = false; | |
| let copyErrorFailed = false; | |
| let copyResetTimeout: ReturnType<typeof setTimeout> | undefined; | |
| let modalOpen = false; | |
| onDestroy(() => { | |
| if (copyResetTimeout) { | |
| clearTimeout(copyResetTimeout); | |
| } | |
| }); | |
| $: networkList = Array.from(networkStatuses.values()); | |
| $: hasNetworks = networkList.length > 0; | |
| $: hasFailure = networkList.some((s) => s.status === 'failure'); | |
| $: displayStatus = (hasFailure ? 'failure' : 'active') as LocalDbStatus; | |
| const copyError = async () => { | |
| if (!error || typeof navigator === 'undefined' || !navigator.clipboard?.writeText) { | |
| return; | |
| } | |
| if (copyResetTimeout) { | |
| clearTimeout(copyResetTimeout); | |
| copyResetTimeout = undefined; | |
| } | |
| copyErrorFailed = false; | |
| try { | |
| await navigator.clipboard.writeText(error); | |
| copied = true; | |
| copyResetTimeout = setTimeout(() => { | |
| copied = false; | |
| copyResetTimeout = undefined; | |
| }, 2500); | |
| } catch { | |
| copied = false; | |
| copyErrorFailed = true; | |
| } | |
| }; | |
| function openModal() { | |
| modalOpen = true; | |
| } | |
| export let networkStatuses: Map<number, NetworkSyncStatus> = new Map(); | |
| export let orderbookStatuses: Map<string, OrderbookSyncStatus> = new Map(); | |
| let modalOpen = false; | |
| $: networkList = Array.from(networkStatuses.values()); | |
| $: hasNetworks = networkList.length > 0; | |
| $: hasFailure = networkList.some((s) => s.status === 'failure'); | |
| $: hasSyncing = networkList.some((s) => s.status === 'syncing'); | |
| $: displayStatus = (hasFailure ? 'failure' : hasSyncing ? 'syncing' : 'active') as LocalDbStatus; | |
| function openModal() { | |
| modalOpen = true; | |
| } |
🤖 Prompt for AI Agents
In packages/ui-components/src/lib/components/LocalDbStatusCard.svelte around
lines 11 to 23, the derived displayStatus only checks for 'failure' and
otherwise returns 'active', which hides ongoing 'syncing' activity; change the
derivation to compute hasFailure = networkList.some(s => s.status ===
'failure'), hasSyncing = networkList.some(s => s.status === 'syncing'), and set
displayStatus = (hasFailure ? 'failure' : hasSyncing ? 'syncing' : 'active') as
LocalDbStatus so failure takes precedence, syncing is shown when present,
otherwise active.
Collaborator
Author
There was a problem hiding this comment.
yes this is intentional. we have syncing status available for every network in the modal that we are using
Contributor
There was a problem hiding this comment.
@findolor, thank you for the clarification! That makes perfect sense. Cards presenting a summary and linking to additional details is a well-established UI pattern. Your approach of showing a high-level status (failure/active) in the card while exposing granular per-network sync phases in the modal provides a clean separation of concerns and avoids overwhelming users with too much information at a glance.
🧠 Learnings used
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.
| @@ -1,5 +1,5 @@ | |||
| const REMOTE_SETTINGS_URL = | |||
| 'https://raw.githubusercontent.com/rainlanguage/rain.strategies/22d29212502af6c796912516a19fba536981c471/settings.yaml'; | |||
| 'https://raw.githubusercontent.com/rainlanguage/rain.strategies/160d2c7119d6423c22737db1eb9ca71190120775/settings.yaml'; | |||
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify URL accessibility and YAML validity
echo "=== Testing URL Accessibility ==="
STATUS=$(curl -s -o /dev/null -w "%{http_code}" \
'https://raw.githubusercontent.com/rainlanguage/rain.strategies/160d2c7119d6423c22737db1eb9ca71190120775/settings.yaml')
echo "HTTP Status: $STATUS"
if [ "$STATUS" = "200" ]; then
echo "✓ URL is accessible"
echo ""
echo "=== Fetching Remote Settings Content ==="
curl -s 'https://raw.githubusercontent.com/rainlanguage/rain.strategies/160d2c7119d6423c22737db1eb9ca71190120775/settings.yaml'
else
echo "❌ URL returned status $STATUS"
fiRepository: rainlanguage/rain.orderbook
Length of output: 5074
🏁 Script executed:
#!/bin/bash
# Examine the loadRemoteSettings.ts file structure
echo "=== File: tauri-app/src/lib/services/loadRemoteSettings.ts ==="
cat -n tauri-app/src/lib/services/loadRemoteSettings.ts
echo ""
echo "=== File: tauri-app/src/lib/services/loadRemoteSettings.test.ts (if exists) ==="
if [ -f "tauri-app/src/lib/services/loadRemoteSettings.test.ts" ]; then
wc -l tauri-app/src/lib/services/loadRemoteSettings.test.ts
head -50 tauri-app/src/lib/services/loadRemoteSettings.test.ts
else
find tauri-app -name "*loadRemoteSettings*test*" -o -name "*loadRemoteSettings*.test.*"
fiRepository: rainlanguage/rain.orderbook
Length of output: 3035
🏁 Script executed:
#!/bin/bash
# Search for how REMOTE_SETTINGS_URL is used in the codebase
echo "=== Usage of REMOTE_SETTINGS_URL ==="
rg "REMOTE_SETTINGS_URL|loadRemoteSettings" tauri-app --type ts --type tsx -B 2 -A 2Repository: rainlanguage/rain.orderbook
Length of output: 152
Remote settings URL is accessible and structure is compatible with per-network configuration.
The URL returns valid YAML (HTTP 200) containing networks, subgraphs, orderbooks, deployers, and per-network sync settings that support the per-network status handling introduced in this PR. The function implementation is solid with comprehensive test coverage for success and error paths.
Consider documenting in the commit message or PR description why this specific commit (160d2c7) was chosen rather than a mutable branch reference, to aid future maintenance and context.
🤖 Prompt for AI Agents
In tauri-app/src/lib/services/loadRemoteSettings.ts around line 2, the hardcoded
remote settings URL pins a specific commit hash (160d2c7) but lacks contextual
documentation explaining why a fixed commit was chosen over a branch reference;
update the repository by adding a short explanatory comment above this line (or
include a clear note in the PR/commit message) stating that the immutable commit
hash was intentionally used to guarantee stable, reproducible settings for this
release and to prevent unexpected changes from a moving branch, and briefly
mention when/how to update the hash in future (e.g., update to a new verified
commit and bump version).
4 tasks
…nular-network-status-ui
hardyjosh
approved these changes
Dec 29, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Chained PRs
Dependent PRs
Motivation
See issues:
The current Local DB status UI only exposes a single global status (active/syncing/failure). Users can't tell which network is syncing or failing, what step the sync is currently in (fetching logs, decoding, persisting, etc.), or whether progress is being made. This makes troubleshooting difficult and contributes to "syncing forever" confusion.
Additionally, the sync engine already emits useful phase messages via the
StatusBustrait, but the client-side status bus (ClientStatusBus) was previously a no-op, so those phase updates never reached the UI.Solution
This PR provides granular, per-network visibility into local sync by implementing:
Rust Core Changes
FetchingLogs,Decoding,Persisting, etc.) with wasm-bindgen support to replace string-based phase messagesSyncPhaseinstead of stringsStore Layer
(chainId, orderbookLabel)UI Components
Testing
Checks
By submitting this for review, I'm confirming I've done the following:
fix #2379
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.