fix(editor): a drum edit no longer strips authored keys from drum-part manifest entries - #345
Merged
Merged
Conversation
…t manifest entries
The /save_song drum block rebuilds every type:"drums" pointer entry
from scratch as {id, name, type, drum_tab} whenever the client ships
drum_parts (any drum-dirty save). A pack whose drum entries carry
additive spec fields the editor doesn't author — feedpak-spec 1.18.0's
per-arrangement `tones` sound binding (spec #62), or any extension key
— lost them silently on the next drum edit + save.
The rebuild now merges each entry ONTO its prior same-id entry through
the same _merge_manifest_entry rule the pitched pipeline has followed
since the merge-not-rebuild save: authored/additive keys survive
verbatim, the editor-owned keys always take the rebuilt value, and a
part with no predecessor (or no id match after a rename) yields exactly
the clean four-key entry — so single-drum packs stay byte-identical.
Extracted as module-level _drum_pointer_entry for pytest, mirroring how
the create-mode twin (_create_build_drum_entries) is pinned. Create
mode itself builds fresh packs with no prior manifest, so it has
nothing to preserve and is untouched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughDrum arrangement pointer entries rebuilt during save now preserve prior additive keys while replacing editor-owned fields. The change adds a merge helper, integrates it into primary and extra drum pointer reconstruction, adds regression tests, and documents the fix. ChangesDrum manifest preservation
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
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.
What
Closes a data-loss window opened by feedpak-spec 1.18.0 (spec #62, merged today): a
type: "drums"arrangement entry can now carry a per-arrangementtonessound binding — but the editor's/save_songdrum block rebuilds every drum-pointer entry from scratch as{id, name, type, drum_tab}on any drum-dirty save. Edit the drums in a pack whose entries carrytones(or any extension key), hit Save, and the binding silently vanished.The fix
The rebuild now merges each entry onto its prior same-id entry through the existing
_merge_manifest_entry— the exact unknown-key preservation rule the pitched pipeline has followed since the merge-not-rebuild save (#101). Semantics:tones, extension keys) survive verbatim;id/name/type/drum_tab) always take the rebuilt value;Extracted as module-level
_drum_pointer_entry(used at both the primary-alias and extra-part sites), mirroring how the create-mode twin_create_build_drum_entriesis pinned. Create mode builds fresh packs with no prior manifest — nothing to preserve, untouched. The absent-drum_partsbranch already re-appends prior entries verbatim, so old clients were never the problem.Spec cross-refs: 1.18.0 §5.2 entry
tones; the §5.1drum_tones/entry-tonesprecedence rule (top-leveldrum_toneswas never at risk — the save merges the manifest top level rather than rebuilding it).Tests
tests/test_drum_parts.py+3 (suite 14/14; the import fails onmainwhere the helper doesn't exist):tones+ an extension key survive a rebuild whilename/drum_tabtake the new values; the prior entry object is never mutatedFull gates: pytest 381/0,
npm run lint0 errors,npm test303/305 (the 2 failures pre-exist onmain).🤖 Generated with Claude Code
https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix
Summary by CodeRabbit