feat(sync): rfc-003 phase a.4.3 — retire payload_hash + pin core to A.4.2 - #58
Conversation
….4.2
Phase A.4.3 of RFC-003 closes the desktop ↔ server payload-hash
loop. The helper module that landed in waveflow-server A.2.2.1
(`src/payload_hash.rs`, 366 LOC, 12 inline tests) moves into
waveflow-core under `sync::payload_hash` (desktop PR #238 — A.4.1)
so a single canonical-serialisation implementation drives both
sides of the wire. The server bumps its `waveflow-core` pin to
the desktop main SHA that ships both A.4.1 + A.4.2 and consumes
the helpers from there.
## Cargo
- `waveflow-core` pin bumped to
`ca0819dd5d6a044cea0c3855a3b80b5785ce6c99` — the desktop main
HEAD post-A.4.2 merge, which carries the new
`crates/core/src/sync/{mod.rs,payload_hash.rs}` module + the
`Hlc { wall: i64, logical: i32 }` struct.
## Code
- Delete `src/payload_hash.rs` + remove `pub mod payload_hash;`
from `src/lib.rs`. All consumers (apply pipeline) now `use
waveflow_core::sync::payload_hash::compute_payload_hash;`.
- Add `impl From<crate::sync::Hlc> for waveflow_core::sync::Hlc`
in `src/sync.rs`. The server-side `Hlc` keeps its
`utoipa::ToSchema` derive for the OpenAPI surface; the core
twin is structurally identical (same two fields, same Eq) but
axum-agnostic. Every call site that hands `stamp.hlc` to a
core helper gets `.into()` so the conversion stays explicit at
the boundary.
## Hors scope
- ❌ Backfill du `payload_hash` sur rows legacy server → Phase B
- ❌ Apply pipeline desktop (consumer) → Phase B (WS subscriber)
- ❌ CRDT OR-Set + FI → Phase C
- ❌ Lamport retire → Phase D
- ❌ Cargo workspace consolidation (server + core in one repo) —
out of scope, the git-pin approach stays for now
## Validation
- `cargo check --all-targets --all-features` clean.
- `cargo clippy --all-targets --all-features -- -D warnings` clean.
- `cargo test --lib` — 34 pass (the 12 ex-payload_hash unit tests
now live in waveflow-core and run under `cargo test
-p waveflow-core`).
- `cargo test --test apply --test apply_digest` — 37 + 7 pass.
- `cargo test --test sync` — 17 pass (push v1/v2 round-trip, HLC
regression, websocket fan-out, etc.).
The byte-for-byte stability of `compute_payload_hash` between
the retired server module and the core port is asserted by the
A.4.1 PR's port test suite (#238 commit `2c20bd9` — 12 tests
mirror the server's exact byte-form expectations).
Refs: RFC-003 §2 (total order), §4 (canonical form). Closes
the Phase A.4 cross-repo loop opened by desktop #238 + #244.
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
Warning Review limit reached
More reviews will be available in 1 hour, 19 minutes, and 32 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughLe module local ChangesMigration payload_hash vers waveflow_core
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 unit tests (beta)
Comment |
github-actions
Bot
added
scope: server
|
@coderabbitai full review please. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== PR `#58` diff ==="
gh pr diff 58 --repo InstaZDLL/waveflow-server
echo ""
echo "=== Changed files list ==="
gh pr view 58 --repo InstaZDLL/waveflow-server --json files --jq '.files[].path'Length of output: 54461
Revue — Phase A.4.3✅ Logique du changement — OKLa migration est propre :
🔴 Problème critique : explosion transitive des dépendancesLe Le serveur est configuré avec Conséquences concrètes :
Ce qu'il faut vérifier dans 🟡 Observation mineure
Résumé : La mécanique de migration est correcte. Le bloquant est l'entrée non intentionnelle de wasmtime dans le graphe de dépendances du serveur — à résoudre dans ✅ Action performedFull review finished. |
github-actions
Bot
added
type: feat
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
github-actions
Bot
added
type: feat
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 `@Cargo.toml`:
- Line 26: The waveflow-core dependency in Cargo.toml specifies a rev hash that
is not accessible on the upstream GitHub repository, compromising
reproducibility. Additionally, the resolved Cargo.lock includes unnecessary
WASM-related dependencies (wasmtime, wasmtime-wasi, waveflow-plugin-sdk) despite
disabling default features and only enabling postgres, adding unnecessary binary
bloat and compilation overhead. Verify that the specified rev commit hash exists
and is accessible on the upstream waveflow-core repository. If the rev is
invalid, either contact the waveflow-core maintainers to request feature-gating
the WASM plugin stack as optional dependencies, or revert the waveflow-core
dependency to a previous known-good rev that does not include these unnecessary
WASM dependencies, and block further upgrades until the upstream project
properly isolates optional plugin functionality.
In `@src/apply.rs`:
- Around line 298-299: The code in src/apply.rs fails the Rust formatting check
(cargo fmt --all --check). The import statement for compute_payload_hash and the
surrounding code wrapping around compute_payload_hash calls do not conform to
rustfmt standards. Run cargo fmt --all to automatically fix all formatting
issues in the file, then verify the changes pass cargo fmt --all --check before
committing.
🪄 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: 97382e36-7e59-4b49-8829-8a9c9790b624
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!Cargo.lock,!*.lock
📒 Files selected for processing (5)
Cargo.tomlsrc/apply.rssrc/lib.rssrc/payload_hash.rssrc/sync.rs
💤 Files with no reviewable changes (2)
- src/payload_hash.rs
- src/lib.rs
Follow-up to #58 — CodeRabbit flagged two rustfmt diffs the .into() add introduced: - src/apply.rs:1506 — import order: `use crate::sync::SyncOpIn` must precede `use waveflow_core::sync::payload_hash::*` because rustfmt groups crate-local imports above external-crate ones. - src/apply.rs:1699 — the `.into()` push past 100 chars triggered the standard binary-rhs wrap. `cargo fmt --all -- --check` clean post-fix. No behavioural change; pure style. Pin SHA stays untouched — desktop PR #245 will land first to remove the wasmtime + Cranelift transitive deps that CodeRabbit's other finding flagged, then a separate commit rebases the pin onto its merge SHA. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
github-actions
Bot
added
type: feat
|
@coderabbitai j'ai vérifié les deux findings : 🟡 rev SHA 🔴 wasmtime/Cranelift bloat transitif — finding 100% valide, confirmé : 🟡 rustfmt apply.rs — fix appliqué sur commit Validation : |
|
Tip For best results, initiate chat on the files or code changes.
Ce PR peut merger dès que WaveFlow#245 est fusionné et le pin rebumpé avec le lock régénéré. Rien d'autre à signaler côté serveur. 🐇 |
1 participant
Summary
Phase A.4.3 of RFC-003 closes the desktop ↔ server payload-hash
loop. The helper module that landed here in A.2.2.1
(
src/payload_hash.rs, 366 LOC, 12 inline tests) moves intowaveflow-coreundersync::payload_hash(desktop PR #238 —A.4.1) so a single canonical-serialisation implementation drives
both sides of the wire. This PR bumps the
waveflow-corepin tothe desktop main SHA that ships both A.4.1 + A.4.2 and consumes
the helpers from there.
Cargo
waveflow-corepin bumped toca0819dd5d6a044cea0c3855a3b80b5785ce6c99— the desktop main HEAD post-A.4.2 merge, carries the newcrates/core/src/sync/{mod.rs,payload_hash.rs}module + theHlc { wall: i64, logical: i32 }struct.Code change
src/payload_hash.rs+ removepub mod payload_hash;fromsrc/lib.rs. All consumers (apply pipeline) nowuse waveflow_core::sync::payload_hash::compute_payload_hash;.impl From<crate::sync::Hlc> for waveflow_core::sync::Hlcinsrc/sync.rs. The server-sideHlckeeps itsutoipa::ToSchemaderive for the OpenAPI surface; the core twin is structurally identical (same two fields, same Eq) but axum-agnostic. Every call site that handsstamp.hlcto a core helper gets.into()so the conversion stays explicit at the boundary.What this does NOT do
payload_hashsur rows legacy server → Phase BTest plan
cargo check --all-targets --all-featurescleancargo clippy --all-targets --all-features -- -D warningscleancargo test --lib— 34 pass (the 12 ex-payload_hash unit tests now live inwaveflow-coreand run undercargo test -p waveflow-core)cargo test --test apply --test apply_digest— 37 + 7 pass (apply pipeline + digest endpoint end-to-end)cargo test --test sync— 17 pass (push v1/v2 round-trip, HLC regression, websocket fan-out)Byte-for-byte stability of
compute_payload_hashbetween the retiredserver module and the core port is locked down by the A.4.1 PR's
suite (desktop #238 commit
2c20bd9— 12 tests mirror the server'sexact byte-form expectations).
Refs
waveflow_core::sync::payload_hashmodule landedCloses the Phase A.4 cross-repo loop opened by desktop #238 + #244.
✅ This is the last PR of Phase A.
Summary by CodeRabbit
Release Notes
Chores
waveflow-coreafin de bénéficier des dernières améliorations.Bug Fixes
Refactor
waveflow-core.