feat(sync): rfc-003 phase a.1.2 — entity tables hlc + payload_hash + metadata_digest_version - #51
Conversation
|
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 (1)
📝 WalkthroughWalkthroughCette PR ajoute une migration SQL qui introduit colonnes HLC (hlc_wall, hlc_logical), origin_device_id et payload_hash sur sept tables, et crée la table metadata_digest_version (clé composite profile_id+entity, version monotone). ChangesMigration schéma HLC et versioning
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
…metadata_digest_version Phase A.1.2 of the RFC-003 sync v2 rollout. Additive schema-only change that gives every materialised sync entity the (hlc_wall, hlc_logical, origin_device_id, payload_hash) tuple the apply pipeline will start populating in Phase A.2, and ships the metadata_digest_version counter table that backs the §metadata_digest_version invariant. Tables touched: profile, library, track, playlist, playlist_track, user_liked_track, user_track_rating. Album/artist/track_artist stay untouched (auto-materialised, HLC piggybacks on the source track row); library_folder doesn't exist server-side yet (lands in Phase C). hlc_wall + hlc_logical default to (0, 0) so the ALTER TABLE doesn't need a separate UPDATE pass and any future v2 op strictly outranks the legacy materialised state under §2's total order. origin_device_id + payload_hash stay nullable — there's no honest pre-A.1.2 value for either, and Phase A.2 tightens them per-entity once every row has round-tripped through the apply path. No behaviour change: pure schema add, validated by the existing test suite (32 tests) running against a fresh PG with this migration applied alongside A.1.1. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
0c3f645 to
07d2289
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@migrations/20260613000000_entity_hlc.sql`:
- Line 19: The migration comment for hlc_logical is inconsistent with the
implementation: change the description of `hlc_logical` from "u32-shaped
per-tick counter" to "i32-shaped per-tick counter" so it matches the validation
in src/db.rs (range 0..=i32::MAX) and the existing comment about "INTEGER (i32)
per the RFC §2 definition"; update any adjacent wording to reflect a signed
32-bit type.
🪄 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: 6e597425-2b5b-48f2-a089-76d76b8f9ac2
📒 Files selected for processing (1)
migrations/20260613000000_entity_hlc.sql
Wording fix flagged in #51 review: "u32-shaped per-tick counter" was misleading — Postgres INTEGER is signed 32-bit (i32, ceiling 2^31-1), which is also what src/db.rs's `0..=i32::MAX` range check enforces. Aligns with the existing "INTEGER (i32) per the RFC §2 definition" comment in db.rs:121. No schema change.
What
Phase A.1.2 of the RFC-003 sync v2 rollout. Schema-only additive change that gives every materialised sync entity the
(hlc_wall, hlc_logical, origin_device_id, payload_hash)tuple the apply pipeline will start populating in Phase A.2, plus themetadata_digest_versioncounter table that backs the §metadata_digest_version invariant.Follows #50 (A.1.1 — same shape on
sync_op).Migration
migrations/20260613000000_entity_hlc.sql:ALTER TABLE … ADD COLUMNon profile, library, track, playlist, playlist_track, user_liked_track, user_track_rating:hlc_wall BIGINT NOT NULL DEFAULT 0hlc_logical INTEGER NOT NULL DEFAULT 0origin_device_id UUID(nullable — no honest pre-A.1.2 value)payload_hash BYTEA(nullable — recomputed by the apply path at write time)CREATE TABLE metadata_digest_version (profile_id BIGINT, entity TEXT, version BIGINT, PK (profile_id, entity))withON DELETE CASCADEfromprofile.What this does NOT do
add_at_*/delete_at_*OR-Set tombstone columns onplaylist_track/user_liked_track— those land in Phase C alongside the OR-Set semantics activation (RFC §3).album/artist/track_artist— auto-materialised, HLC piggybacks on the sourcetrackrow.library_folderHLC — table doesn't exist server-side yet, lands in Phase C.metadata_digest_versiononliked_track/track_rating— they aren't profile-keyed, the routing decision (per-user pseudo-profile vs sibling table) is deferred to A.2 when the digest endpoint wiring lands. Table exists now so A.2 has a target to bump against without a follow-up migration.origin_device_id/payload_hashto NOT NULL — standard "land schema now, tighten in a follow-up once every row has round-tripped" two-step.Test plan
cargo check --all-targets --all-features— cleancargo clippy --all-targets --all-features -- -D warnings— cleancargo test --all-features— 32 tests pass (apply pipeline, sync, tracks, websocket, compaction, tenants, etc.)The new schema applies cleanly alongside A.1.1's
sync_opcolumns.Refs
Summary by CodeRabbit