feat(sync): rfc-003 phase b.2 — entity fetch-by-canonical endpoint - #66
Merged
Merged
Conversation
`GET /api/v1/sync/entity?entity=X&canonical_id=Y[&profile_canonical_id=Z]`
returns the FULL canonical-fields state of a single materialised row,
so the desktop's backfill orchestrator (RFC-003 Phase B.2) can apply
or merge it under §2 LWW. Counterpart of the digest endpoint: where
digest hands back a hashed snapshot of an entity set, this one
serves one row at a time keyed on its canonical id.
## Per-entity contracts
| Entity | Canonical id | Source tables | `fields` shape |
|----------------|---------------------------------------------|-------------------------------------|----------------|
| `library` | UUID | `library` | 4 keys (mirror `apply::library::canonical_fields`) |
| `playlist` | UUID | `playlist` | 4 keys (mirror `apply::playlist::canonical_fields`) |
| `track` | `<lib_canonical>\u{1F}<file_path>` composite | `track + library + album + artist + track_artist` | 18 keys (mirror `apply::track::canonical_fields`) |
| `liked_track` | `file_hash` | `user_liked_track` | empty `{}` (binary state) |
| `track_rating` | `file_hash` | `user_track_rating` | 1 key `{rating: i64}` |
Track sub-selects on `track_artist` so the multi-artist array
matches the apply-side `ARRAY_AGG(ar.name ORDER BY ta.position)`
shape byte-exact, which the desktop can then feed back through
`compute_payload_hash` to verify against the returned `payload_hash`.
## Scope discipline
Same shape as `GET /api/v1/sync/digest`:
- `library` / `playlist` / `track` require `profile_canonical_id`.
- `liked_track` / `track_rating` reject `profile_canonical_id` (the
endpoint returns 400 if it's present).
- Unknown entities → 400.
- Track composite missing the `\u{001F}` separator → 400 BEFORE the
profile resolve so a structural payload bug isn't masked as 404
("we just don't have that row").
## Other responses
- 404 — `profile_canonical_id` not visible to this user, or the
row exists but its `payload_hash` is still NULL (pre-B.0 stamp).
Both cases are indistinguishable on purpose, mirroring the digest
endpoint's "invisible until stamped" set membership.
- 401 — missing or invalid bearer.
- 500 — DB or internal failure.
## New helpers under `db::entity_read`
- `resolve_profile_id` re-exports the digest module's tenancy
resolver so the API dispatcher doesn't import both submodules
for the same SELECT.
- 5 per-entity `fetch_*` functions, each returning
`Option<EntityFetchResponse>`. Filters mirror `digest_read`:
`payload_hash IS NOT NULL` (and `canonical_id IS NOT NULL` for
profile-scoped) keep the visible set consistent with the digest
sweep.
- `cstr` / `copt_str` / `ci64` / `copt_i64` / `cbool` / `cstrings`
inline canonical-field inserters. Mirror byte-exact of
`apply::canon::*` (and the desktop's
`waveflow_core::sync::canon`). Kept local so the existing
pre-B.0a `waveflow-core` pin doesn't need to move just for the
same 5 1-liners — a future PR bumps the pin and these can switch
to the imported helpers without changing behaviour.
## Tests
10 integration tests in `tests/apply_entity.rs`:
- 5 per-entity round-trip tests (push → fetch → verify
`fields` + `payload_hash` shape).
- Scope discipline matrix (3 cases).
- Track-specific edge cases (composite parsing, missing separator
returns 400).
- 404 paths (unknown canonical_id, cross-tenant isolation).
All 10 pass against the `--all-features` integration harness.
`cargo clippy --all-targets --all-features -- -D warnings` clean.
Refs: RFC-003 §4 (backfill protocol), desktop PR #249 (B.1 digest
client + diff — this endpoint is what the diff's `missing_locally`
+ `divergent` outcomes will resolve through)
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
Warning Review limit reached
More reviews will be available in 1 hour, 52 minutes, and 45 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 (3)
📝 WalkthroughWalkthroughAjout de l'endpoint ChangesEndpoint GET /api/v1/sync/entity – read-path complet
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
github-actions
Bot
added
type: feat
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/api/sync.rs`:
- Around line 716-746: The docstring for the `get_entity` handler function is
unclear about entity coverage. Currently it states "Same scope discipline as
[`get_digest`]" which only describes the `profile_canonical_id` requirement
rules, not which entities are actually supported. Update the docstring to
explicitly clarify that unlike `/digest`, the `/entity` endpoint does not
support the `profile` entity. List the actually supported entities (`library`,
`playlist`, `track`, `liked_track`, `track_rating`) in the documentation and
make clear that `profile` is intentionally excluded by design.
In `@src/db.rs`:
- Around line 1528-1532: The Rust code in multiple files fails the cargo fmt
formatting checks. Run `cargo fmt --all` from the project root to automatically
fix all formatting violations across the codebase. This will fix formatting
issues in the copt_str function at src/db.rs lines 1528-1532, the
album_artist_name invocation at src/db.rs lines 1721-1727, and the fetch
function signature at tests/apply_entity.rs lines 67-79. All three locations
should be corrected by this single formatting command.
🪄 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: c99f3a35-f108-4060-941c-b0c29341d20d
📒 Files selected for processing (4)
src/api/sync.rssrc/db.rssrc/sync.rstests/apply_entity.rs
Comment on lines
+1528
to
+1532
| fn copt_str(m: &mut Map<String, Value>, k: &str, v: Option<&str>) { | ||
| m.insert( | ||
| k.to_owned(), | ||
| v.map(|s| Value::String(s.to_owned())).unwrap_or(Value::Null), | ||
| ); |
There was a problem hiding this comment.
Échecs de formatage Rust (cargo fmt) dans plusieurs fichiers.
Le pipeline CI échoue car le code n'est pas formaté selon les conventions Rust. Exécute cargo fmt --all pour corriger tous les fichiers d'un coup.
src/db.rs#L1528-L1532: formatage de la fonctioncopt_str(.unwrap_or(Value::Null)).src/db.rs#L1721-L1727: formatage de l'invocation multi-ligne pouralbum_artist_name.tests/apply_entity.rs#L67-L79: formatage de la signature de la fonctionfetch.
🧰 Tools
🪛 GitHub Actions: CI (Rust) / 0_Rust (ubuntu-latest).txt
[error] 1528-1528: cargo fmt --all --check failed due to rustfmt formatting differences in this file (string mapping/unwrap_or formatting). Run 'cargo fmt --all' to apply formatting.
🪛 GitHub Actions: CI (Rust) / Rust (ubuntu-latest)
[error] 1528-1528: cargo fmt --all --check failed (formatting diff). Please run cargo fmt --all to apply Rust formatting changes.
📍 Affects 2 files
src/db.rs#L1528-L1532(this comment)src/db.rs#L1721-L1727tests/apply_entity.rs#L67-L79
🤖 Prompt for 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.
In `@src/db.rs` around lines 1528 - 1532, The Rust code in multiple files fails
the cargo fmt formatting checks. Run `cargo fmt --all` from the project root to
automatically fix all formatting violations across the codebase. This will fix
formatting issues in the copt_str function at src/db.rs lines 1528-1532, the
album_artist_name invocation at src/db.rs lines 1721-1727, and the fetch
function signature at tests/apply_entity.rs lines 67-79. All three locations
should be corrected by this single formatting command.
Source: Pipeline failures
Two CodeRabbit findings on PR #66: 1. `get_entity` docstring under-documented entity coverage — said "same scope discipline as /digest" without flagging that the actual entity set diverges. /digest serves `profile` so the desktop can verify the auto-provisioned row's canonical fields; /entity has no `profile` reader because the canonical id used to address it (`profile_canonical_id`) IS the per- tenant scope identifier — a round-trip on the same id, with the row's HLC + payload_hash already observable from /digest. Rewrote the docstring to enumerate the 5 supported entities + an explicit "intentionally excluded: profile" block with the rationale. 2. `cargo fmt --all --check` flagged 3 sites the manual edits left unindented (the `copt_str` `unwrap_or` chain, the `album_artist_name` invocation, the `fetch` test helper signature). Ran `cargo fmt --all`. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
github-actions
Bot
added
type: feat
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
1 participant
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.
What
GET /api/v1/sync/entity?entity=X&canonical_id=Y[&profile_canonical_id=Z]returns the FULL canonical-fields state of a single materialised row. Counterpart of the digest endpoint: where/sync/digesthands back a hashed snapshot of an entity set, this one serves one row at a time keyed on its canonical id.The desktop's backfill orchestrator (RFC-003 Phase B.2, ship in a follow-up cross-repo PR) consumes this to resolve digest diffs:
missing_locally(server has, desktop doesn't) → fetch + apply locallydivergent(same canonical, hashes differ) → fetch remote, compare §2 HLC tuples, apply the LWW winnerPer-entity contracts
fieldsshapelibrarylibraryapply::library::canonical_fields)playlistplaylistapply::playlist::canonical_fields)track<lib_canonical>\u{1F}<file_path>compositetrack + library + album + artist + track_artistapply::track::canonical_fields)liked_trackfile_hashuser_liked_track{}(binary state)track_ratingfile_hashuser_track_rating{rating: i64}Track sub-selects on
track_artistso the multi-artist array matches the apply-sideARRAY_AGG(ar.name ORDER BY ta.position)shape byte-exact — the desktop can recomputepayload_hashagainst the returned(fields, hlc, origin_device_id)to verify the server hasn't drifted from its own write path.Scope discipline (mirrors
/sync/digest)library/playlist/trackrequireprofile_canonical_id. Missing → 400.liked_track/track_ratingrejectprofile_canonical_id. Present → 400.\u{001F}separator → 400 before the profile resolve so a structural payload bug isn't masked as 404 ("we just don't have that row").Other responses
profile_canonical_idnot visible to this user, or the row exists but itspayload_hashis still NULL (pre-B.0 stamp). Both cases indistinguishable on purpose, mirroring the digest endpoint's "invisible until stamped" set membership.What this does NOT do
playlist_track(nocanonical_fields/payload_hashserver-side per B.0-rest rationale).profile(the profile row itself doesn't need a per-row fetch — its canonical fields live in the digest sweep).New helpers
Test plan
Tests cover:
fieldsshape +payload_hashhex length)Refs
Signed-off-by: InstaZDLL claude.ai.cm9ni@twiceland.world
Summary by CodeRabbit
Notes de version
Nouvelles fonctionnalités
Tests