feat(sync): drain task pushes pending ops to waveflow-server (Phase 1.f.desktop.4a)#196
Conversation
…ktop.4a) Reveille the WaveflowServerClient struct that has been dormant since #189 and pipes the local sync_pending_op queue into POST /api/v1/sync/ops. One-way push for now; WebSocket subscriber + apply remote ops + canonical-id mapping all land in a follow-up 1.f.desktop.4b. ## sync::drain Background task spawned at boot from lib.rs. Loop alternates between a 30 s periodic tick and a tokio::sync::Notify wake fired by CRUD command sites after a successful tx.commit() — a chatty user's edits reach the server within ms instead of waiting the full poll interval. Two gates short-circuit the pass without an HTTP round-trip: 1. WaveflowServerClient::try_build — both the server URL and the active profile JWT must be configured. 2. SyncMode::Hybrid — Local-mode profiles keep their queue local. Either gate yields DrainOutcome::Skipped. ## Failure semantics - HTTP 200 — drop_acked removes the rows, loop continues to drain any newly-arrived ops. - HTTP 409 lamport_regression — lamport::observe_remote bumps the local floor past the server's view, mark_failed on the offending row, break the loop. The next pass retries with the bumped clock. - Other HTTP statuses + network errors — mark_failed the batch with the server reply for diagnostics, break. The periodic poll re-attempts later. ## Tauri surface - New command sync_drain_now for the Settings diagnostics "Push now" affordance — runs drain_once synchronously and returns the DrainOutcome. - sync_set_mode now notifies the drain task on the Hybrid switch so flipping the radio doesn't wait 30 s to fire the first push. ## Boot wiring - AppState gains drain: Arc<DrainHandle>, default-initialised so callers can notify() against it harmlessly before the task spawns. - lib.rs::run spawns the task right after app.manage(state) so app.state::<AppState>() resolves and the same Arc<Notify> is shared by command sites + the task. ## Notify-on-commit in playlist commands All 8 mutation sites in commands/playlist.rs gain a state.drain.notify() right after tx.commit(). No-op when the drain task isn't spawned yet (very brief window between state.manage and drain::spawn). ## Tests - 25 desktop sync unit tests still green (+3 new in sync::drain pinning DrainOutcome's snake_case discriminant tag, PushBatchRequest wire shape vs waveflow-server, and LamportRegression deserialisation from a server 409 body). ## Out of scope (1.f.desktop.4b) - WebSocket subscriber + apply remote ops on local SQLite. - Canonical-id mapping for cross-device entity identity. Today's entity_id is the local i64 coerced to TEXT, which is fine for the push direction since the server keys ops on (user_id, device_id, entity, entity_id) — different devices' ops live in disjoint namespaces. Cross-device replay needs the mapping table the WS subscriber will introduce. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAjoute une tâche background ChangesSynchronisation en arrière-plan avec vidage des opérations en attente
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/crates/app/src/sync/drain.rs`:
- Around line 146-176: The drain_once calls must be serialized to avoid
concurrent drains reading the same pending batch—add a shared async lock (e.g.,
tokio::sync::Mutex or a tokio::sync::Semaphore/OwnedPermit) to AppState (call it
drain_lock or drain_mutex) and acquire it before invoking drain_once in both the
background spawn task (inside spawn where task_handle and ticker are used) and
the manual sync_drain_now handler so only one drain_once runs at a time; ensure
the lock is held across the await of drain_once and released afterwards (drop
the guard) to prevent duplicate sends.
- Around line 249-257: When parsing the 409 body into LamportRegression fails
the code currently does a silent break; instead call the batch failure handler
and log the parsing error. In the error branch where resp.json().await returns
Err(err) (LamportRegression), invoke mark_failed(...) for the current batch (use
the same batch identifier used elsewhere in this function) with a descriptive
reason that includes err, emit a tracing::warn/error with that err, and then
exit/return the handler as appropriate so the failure is recorded (do not just
break silently). Ensure you reference the LamportRegression parsing site and the
mark_failed method when making this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b0557a83-f501-4012-97dd-2bb1c440e9a7
📒 Files selected for processing (7)
src-tauri/crates/app/src/commands/playlist.rssrc-tauri/crates/app/src/commands/sync.rssrc-tauri/crates/app/src/lib.rssrc-tauri/crates/app/src/server_client.rssrc-tauri/crates/app/src/state.rssrc-tauri/crates/app/src/sync/drain.rssrc-tauri/crates/app/src/sync/mod.rs
@coderabbitai surfaced two valid issues on PR #196: 1. drain_once could be called concurrently by the background tick and the sync_drain_now Tauri command. Both would read the same sync_pending_op batch and POST it twice — the server absorbs the duplicates via the operation_id UNIQUE, but the wasted round-trip + duplicated total_sent accounting is avoidable. Added a shared Arc<tokio::sync::Mutex<()>> on AppState (drain_lock). Both call sites acquire it before drain_once and hold the guard across the await; a concurrent caller waits for the in-flight pass to finish rather than racing it. 2. When the 409 body failed to parse as LamportRegression, the loop just broke after a tracing::warn. The rows stayed at attempt_count=0, so the next pass would hit the same 409 forever without any diagnostic trail. Now mark_failed the whole batch with the parse error string before breaking, matching the shape of the other-status and network-error branches. 25/25 sync tests still green, clippy + fmt clean. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
Wakes up the `WaveflowServerClient` struct dormant since #189 and pipes the local `sync_pending_op` queue into `POST /api/v1/sync/ops`. One-way push for now — the WebSocket subscriber + apply-remote-ops + canonical-id mapping all land in a follow-up 1.f.desktop.4b.
`sync::drain` module
Background task spawned at boot from `lib.rs`. Loop alternates between a 30 s periodic tick and a `tokio::sync::Notify` wake fired by CRUD command sites after a successful `tx.commit()` — a chatty user's edits reach the server within ms instead of waiting the full poll interval.
Two gates short-circuit the pass without an HTTP round-trip:
Either gate yields `DrainOutcome::Skipped`.
Failure semantics
Tauri surface
Boot wiring
Notify-on-commit in playlist commands
All 8 mutation sites in `commands/playlist.rs` gain a `state.drain.notify()` right after `tx.commit()`. No-op when the drain task isn't spawned yet (very brief window between `state.manage` and `drain::spawn`).
Test plan
Out of scope (1.f.desktop.4b)
Summary by CodeRabbit
Nouvelles fonctionnalités
Améliorations
Corrections