feat(sync): emit per-track snapshots in playlist tracks ops (phase 1.j.b)#203
Conversation
…j.b) Outbound wire bump that follows server PR #32. Every command that emits a `playlist + field: "tracks", op: "insert"` op now folds a per-track `snapshots` map into the payload alongside `track_ids`, so the server's `playlist_track.snapshot_*` columns land populated and the public share preview at `/api/v1/share/playlists/{token}` can render the title + artist + duration without resolving the local-i64 track id cross-device. Shape: { "track_ids": [42, 43], "snapshots": { "42": { "title": "One More Time", "artist": "Daft Punk", "duration_ms": 320000 }, "43": { "title": "Around the World", "duration_ms": 280000 } } } `artist` is omitted (not null) when no `track_artist` row exists. `duration_ms` is always present. A track id whose row was deleted between the playlist write and the snapshot SELECT is silently dropped from the map; the server tolerates the id-without-snapshot case (row lands NULL-snapshot, invisible to the public preview). Three emit sites bumped — all inside the same SQLite transaction as the playlist write + the outbox enqueue: - `add_track_to_playlist` - `add_tracks_to_playlist` - `add_tracks_from_source` `remove_track_from_playlist` and `reorder_playlist_track` keep their minimal payloads (no display change on the receiving side). New module `sync::track_snapshots` runs the GROUP_CONCAT artist query (mirrors the SELECT in `repository/sqlite/track.rs:33`) and returns the map. Uses `sqlx::AssertSqlSafe` for the `IN (?, ?, …)` expansion, same audited pattern as `commands/radio.rs:225`. `iter::repeat().take(n)` rather than `repeat_n` to respect the 1.80 MSRV. Tests: 5 unit tests on the snapshot builder (empty input, single track + artist, multi-artist join string, missing track id dropped, artist-absent omits the field). 128/128 lib tests pass. Includes scattered `cargo fmt` cleanups on `commands/share.rs`, `db/profile_meta.rs`, `sync/drain.rs` — pure whitespace, picked up by the formatter when I ran it on the snapshot module. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
InstaZDLL
added
scope: backend
|
Warning Review limit reached
More reviews will be available in 7 minutes and 34 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ 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)
📝 WalkthroughWalkthroughCe PR ajoute une couche de métadonnées snapshot pour les pistes insérées dans des playlists. Un nouveau module ChangesTrack snapshots for playlist operations
Note: Plusieurs changements de formatage (share.rs, profile_meta.rs, drain.rs) purement cosmétiques sans impact logique sont omis de cette analyse. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@CLAUDE.md`:
- Line 84: The documented command name is incorrect: the docs reference
add_tracks_from_source but the actual handler in commands/playlist.rs is
add_source_to_playlist; update CLAUDE.md to use the real handler name
(add_source_to_playlist) where the rule discusses calling
sync::track_snapshots::build_snapshots(conn, &track_ids) and folding snapshots
into the outbound payload (so references to playlist_track.snapshot_* and the
snapshot filtering behavior remain accurate), or alternatively rename the
handler to add_tracks_from_source if you prefer code-side consistency—ensure doc
and code use the exact same symbol (add_source_to_playlist or
add_tracks_from_source) so grep and future changes are reliable.
🪄 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: 8698e4ae-12a3-4e33-93e6-a4299eba3836
📒 Files selected for processing (7)
CLAUDE.mdsrc-tauri/crates/app/src/commands/playlist.rssrc-tauri/crates/app/src/commands/share.rssrc-tauri/crates/app/src/db/profile_meta.rssrc-tauri/crates/app/src/sync/drain.rssrc-tauri/crates/app/src/sync/mod.rssrc-tauri/crates/app/src/sync/track_snapshots.rs
CodeRabbit flagged the rule referenced `add_tracks_from_source`, but the actual handler in `commands/playlist.rs:551` is `add_source_to_playlist`. Fix the name so a grep against the doc lands on the real call site. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
@coderabbitai valide, j'avais inversé. Le handler côté code est Le commit message original mentionne aussi le mauvais nom mais je le laisse — le git log est immutable et c'est CLAUDE.md qui sert de doc vivante (grep target). |
|
Tip For best results, initiate chat on the files or code changes.
✏️ Learnings added
🧠 Learnings used |
1 participant
Summary
Outbound wire bump that follows server-side PR #32 (closes the share-preview loop for shared playlists). Every command that emits a `playlist + field: "tracks", op: "insert"` op now folds a per-track `snapshots` map into the payload alongside `track_ids`, so the server's `playlist_track.snapshot_*` columns land populated and the public share preview at `/api/v1/share/playlists/{token}` can render the title + artist + duration without resolving the local-i64 track id cross-device.
Wire shape
{ \"track_ids\": [42, 43], \"snapshots\": { \"42\": { \"title\": \"One More Time\", \"artist\": \"Daft Punk\", \"duration_ms\": 320000 }, \"43\": { \"title\": \"Around the World\", \"duration_ms\": 280000 } } }Backward compat
The change is additive — older servers that don't know the `snapshots` field ignore it and store just the `track_ids`. waveflow-server is on the new shape since PR #32 mergé.
Sites bumped (all inside the existing SQLite tx)
`remove_track_from_playlist` and `reorder_playlist_track` keep their minimal payloads (no display change on the receiving side).
New module
`sync::track_snapshots` runs the GROUP_CONCAT artist query (mirrors `repository/sqlite/track.rs:33`) and returns the map. Uses `sqlx::AssertSqlSafe` for the `IN (?, ?, …)` expansion — same audited pattern as `commands/radio.rs:225`. `iter::repeat().take(n)` rather than the newer `repeat_n` helper because the repo's MSRV is 1.80 (`repeat_n` is stable since 1.82).
Test plan
Follow-up
Part of Sprint 2 of the post-1.g sprint plan validated 2026-06-05.
Includes scattered `cargo fmt` cleanups on `commands/share.rs`, `db/profile_meta.rs`, `sync/drain.rs` — pure whitespace, picked up by the formatter when I ran it on the snapshot module.
Summary by CodeRabbit
Notes de version
Nouvelles fonctionnalités
Refactorisation