Cues: copy between hot and memory; drop a round-trip we do not need - #41
Merged
Merged
Conversation
… not need Closes the Cue Destination row, mostly by working out that half of it is a problem `decks` does not have. **The half that was real** is the sync options `All to hot cue` / `All to memory cue` / `All to hot and memory cue` — the spec's "how you copy hot cues into memory cues wholesale", and the standard Rekordbox workflow, because hot cues do not show on every player and memory cues do. Now a `MirrorCues` cue recipe. `Both` is **idempotent**: a position that already exists as both kinds is left alone, because this is run after every session and a second run must not double the cue list. **The half that is a divergence** is the hidden-duplicate round-trip. Lexicon's internal model has hot cues only, so it collapses memory cues into hot cues on import and must remember what it hid to restore it on sync back. `decks` never imports — it reads `djmdCue` live and shows both kinds as they are. Nothing is collapsed, nothing is hidden, nothing needs restoring; the guarantee holds because the problem never arises. Building the ledger would be machinery for a problem we do not have, and it would have looked like diligence. The per-cue `M` toggle falls out the same way: a cue here already *is* one kind or the other, so "destined to become a memory cue" is a state that cannot exist. **Two silent bugs surfaced while wiring it up**, both in `diff_cues`, neither reachable before: - **A recipe that added a cue had it dropped.** The diff walked the result and skipped anything with no `before` to compare against, so `MirrorCues` produced an empty preview and looked like a no-op. Additions now stage as `TrackAddCue`. - **A cue that changed kind staged nothing.** Nothing diffed the `memory` flag, so hot→memory conversion silently did nothing either. Every previous cue recipe only edits, reorders or deletes — this is the first to add a cue or change a kind, and it found both. An argument for testing the diff against the operation set rather than only per-operation. Per docs/lexicon/01-interop.md §Cue Destination.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThe PR adds ChangesCue mirroring
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant DesktopUI
participant CueRecipePreview
participant MirrorCues
participant diff_cues
participant CueRecipeApply
DesktopUI->>CueRecipePreview: request mirror preview
CueRecipePreview->>MirrorCues: apply selected target
MirrorCues-->>CueRecipePreview: converted and added cues
CueRecipePreview->>diff_cues: compare original and recipe cues
diff_cues-->>DesktopUI: edits, deletions, and additions
DesktopUI->>CueRecipeApply: apply preview
CueRecipeApply-->>DesktopUI: staged cue changes
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cole-hackman
marked this pull request as ready for review
August 8, 2026 06:33
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
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.
Closes the Cue Destination row — mostly by working out that half of it is a problem
decksdoes not have.The half that was real
The sync options
All to hot cue/All to memory cue/All to hot and memory cue— the spec's "how you copy hot cues into memory cues wholesale". This is the standard Rekordbox workflow, because hot cues do not show on every player and memory cues do.Now a
MirrorCuescue recipe.Bothis idempotent: a position that already exists as both kinds is left alone. This is something people run after every session, and a second run doubling the cue list would be a nasty surprise.The half that is a divergence
The row said "no hidden-duplicate model, so the round-trip guarantee does not hold", which sounds like a gap. Reading the spec properly, it is not one.
Lexicon's internal model has hot cues only. It collapses memory cues into hot cues on import, hides the duplicates rather than deleting them, and restores them on sync back. That whole apparatus exists to undo a lossy conversion Lexicon performs on the way in.
decksperforms no such conversion. There is no import step — it readsdjmdCuelive, and a memory cue stays a memory cue. Nothing is collapsed, so nothing is hidden, so there is nothing to restore. The guarantee holds because the problem never arises. Building a hidden-duplicate ledger to satisfy the row would have been machinery for a problem we do not have, and it would have looked like diligence.The per-cue
Mtoggle falls out the same way: it marks a Lexicon cue as destined to become a memory cue on the way out, and indecksa cue already is one kind or the other.Two silent bugs it surfaced
Both in
diff_cues, neither reachable before this operation existed:let Some(orig) = by_id.get(...) else { continue }, so any cue a recipe invented went on the floor.MirrorCuescame back with an empty preview — indistinguishable from the recipe not working. Additions now stage asTrackAddCue.memoryflag, so hot→memory conversion silently did nothing either.Every previous cue recipe only edits, reorders or deletes. This is the first to add a cue or change a kind, and it found both — which is a decent argument for testing the diff against the operation set rather than only per-operation.
Verification
Matrix moves to 58 done / 22 partial / 14 missing / 2 blocked / 16 deferred; Interop & sync is now 8 done / 3 partial.
Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation