Refactor undoable commands - step 1

[cbjeukendrup]

This PR starts a refactor of the UndoStack and the way we perform undoable commands. The aim is to design the interface in such way that it is not possible to use it unsafely.

This PR introduces the main infrastructure to get this refactor started, leaving the old interface available for compatibility. Completing the refactor will involve many steps that can be taken later and in parallel, possibly with the help of AI agents.

Reviewing commit-by-commit is recommended; see the commit messages for details.

Update: I split the PR into multiple smaller PRs, so the changes here are not everything. However, reviewing commit-by-commit is still recommended.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR replaces the legacy UndoCommand/UndoMacro/undo.h design with a transaction-based undo system (UndoableCommand, UndoableTransaction, undostack). It migrates editing commands and filter/cleanup signatures, updates Score/MasterScore command lifecycle and update flow (including CmdState deferred deletions), adapts callers across rendering, readers, notation, QML, and tests, and updates build sources and includes to the new transaction headers.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

[coderabbitai]

Actionable comments posted: 6

🧹 Nitpick comments (2)

src/engraving/editing/transaction/undostack.h (1)

136-148: ⚡ Quick win

Const UndoStack accessors currently expose mutable transactions, weakening the safety boundary.

Returning UndoableTransaction* from const methods allows external mutation through a const stack reference. For a safety-oriented API refactor, prefer const UndoableTransaction* for these accessors (or split mutable/non-mutable access paths explicitly).

Suggested header adjustment

-    UndoableTransaction* activeTransaction() const { return m_activeTransaction; }
+    const UndoableTransaction* activeTransaction() const { return m_activeTransaction; }

-    UndoableTransaction* last() const { return m_currentIndex > 0 ? m_transactions[m_currentIndex - 1] : nullptr; }
-    UndoableTransaction* prev() const { return m_currentIndex > 1 ? m_transactions[m_currentIndex - 2] : nullptr; }
-    UndoableTransaction* next() const { return canRedo() ? m_transactions[m_currentIndex] : nullptr; }
+    const UndoableTransaction* last() const { return m_currentIndex > 0 ? m_transactions[m_currentIndex - 1] : nullptr; }
+    const UndoableTransaction* prev() const { return m_currentIndex > 1 ? m_transactions[m_currentIndex - 2] : nullptr; }
+    const UndoableTransaction* next() const { return canRedo() ? m_transactions[m_currentIndex] : nullptr; }

-    UndoableTransaction* lastAtIndex(size_t idx) const
+    const UndoableTransaction* lastAtIndex(size_t idx) const

🤖 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/engraving/editing/transaction/undostack.h` around lines 136 - 148, Const
accessor methods in UndoStack return mutable UndoableTransaction* allowing
mutation via const stacks; change their return types to const
UndoableTransaction* (for activeTransaction(), last(), prev(), next(),
lastAtIndex()) so a const UndoStack only exposes const transactions, and ensure
uses expect const pointers or provide separate non-const overloads if callers
need mutation; update any call sites that assumed mutable pointers to cast to
non-const via non-const stack accessors or to use the new overloads, and keep
references to m_transactions and canRedo() intact.

src/engraving/editing/cmd.cpp (1)

125-152: ⚡ Quick win

Avoid const-ref + std::move in buildScoreChanges to prevent full copies.

On Lines 149-152, changes is a const reference, so those std::move(...) calls copy the containers instead of moving them. This path runs for undo/redo and end-command notifications, so the extra copying can be expensive.

♻️ Proposed fix

-static ScoreChanges buildScoreChanges(const CmdState& cmdState, const UndoableTransaction::ChangesInfo& changes)
+static ScoreChanges buildScoreChanges(const CmdState& cmdState, UndoableTransaction::ChangesInfo changes)
 {
@@
-    return { startTick, endTick,
+    return { startTick, endTick,
              cmdState.startStaff(), cmdState.endStaff(),
              changes.isTextEditing,
              std::move(changes.changedObjects),
              std::move(changes.changedObjectTypes),
              std::move(changes.changedPropertyIdSet),
              std::move(changes.changedStyleIdSet) };
 }

-    ScoreChanges result = buildScoreChanges(cmdState(), changes);
+    ScoreChanges result = buildScoreChanges(cmdState(), std::move(changes));

🤖 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/engraving/editing/cmd.cpp` around lines 125 - 152, buildScoreChanges
currently takes its second parameter as a const
UndoableTransaction::ChangesInfo& then calls std::move(...) on its members,
which forces copies; change the function to accept changes by value
(UndoableTransaction::ChangesInfo changes) so the members can be moved into the
ScoreChanges without extra copies (or alternatively make a local non-const copy
and move from it). Update the buildScoreChanges signature and any call sites
that pass an lvalue accordingly; the symbols to edit are buildScoreChanges, its
parameter type UndoableTransaction::ChangesInfo, and the moved members
changes.changedObjects, changes.changedObjectTypes,
changes.changedPropertyIdSet, and changes.changedStyleIdSet.

🤖 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/engraving/dom/stafftextbase.cpp`:
- Around line 98-105: The score() pointer should be captured before calling
TextBase::removed() to avoid losing the parent Score and thus skipping
updateSwing(); modify StaffTextBase::removed() so it stores Score* s = score()
first, then calls TextBase::removed(), and afterwards calls s->updateSwing()
only if s and swing() are true (i.e., move the existing score capture to before
TextBase::removed()).

In `@src/engraving/editing/editenharmonicspelling.cpp`:
- Around line 274-277: The code that records the winning mask uses idx = i * -1
which yields 0 for i == 0 and later causes the tab1 branch to be chosen
incorrectly; change the negative-index encoding to idx = -(i + 1) when recording
a pb win so zero is never reused, and update the later branch checks (the
tab1/tab2 selection logic around the idx checks in the block handling lines
~318-323) to treat any idx < 0 as a tab2 win (and decode i as -(idx) - 1) rather
than relying on zero/positive semantics so the tab2 selection is preserved when
the best mask is zero.
- Around line 497-499: The code checks keyIndex against Key::NUM_OF but still
unconditionally calls tpcs.front(), which can crash if tpcs is empty; add a
guard such as if (tpcs.empty()) return {} (or otherwise return an appropriate
default/early-error) before the existing if (keyIndex < 0 || keyIndex >=
int(Key::NUM_OF)) block so that you never call tpcs.front() on an empty vector;
update the logic around keyIndex, Key::NUM_OF and tpcs.front() to use the empty
check first.

In `@src/engraving/editing/transaction/undostack.cpp`:
- Around line 214-216: The code deletes m_activeTransaction when rollback is
true, which discards already-applied commands instead of reverting their
effects; instead, invoke the transaction's rollback/undo routine to reverse
applied commands (e.g. call a method like m_activeTransaction->rollback() or
m_activeTransaction->undoAll(), or iterate its command list and call undo() in
reverse order) and only then delete m_activeTransaction; ensure this uses the
same command/undo interfaces used by pushAndPerform() so score mutations are
properly reverted before deleting the transaction.
- Around line 157-162: In pushWithoutPerforming (UndoStack) if
m_activeTransaction is null the function returns early but the
ownership-transferred cmd isn't released; modify pushWithoutPerforming to free
or relinquish cmd before returning (e.g. delete cmd or change the
parameter/local to a std::unique_ptr and let it go out of scope) so there is no
leak when !m_activeTransaction and ScoreLoad::loading() is false; ensure the fix
references the cmd parameter and the early-return branch that checks
m_activeTransaction.
- Line 242: In reopen() the code calls takeAt/decrements m_currentIndex then
unconditionally does m_states.erase(m_states.begin() + m_currentIndex), which
removes the wrong entry (off-by-one) and desyncs clean/dirty state; change the
erase to remove the element that was actually taken (use the index returned by
takeAt or erase at m_currentIndex + 1 if you decremented before erasing) so the
intended state is removed, or simply remove the extra erase if takeAt already
removed the element—adjust m_states.erase(...) to target the correct index
relative to m_currentIndex.

---

Nitpick comments:
In `@src/engraving/editing/cmd.cpp`:
- Around line 125-152: buildScoreChanges currently takes its second parameter as
a const UndoableTransaction::ChangesInfo& then calls std::move(...) on its
members, which forces copies; change the function to accept changes by value
(UndoableTransaction::ChangesInfo changes) so the members can be moved into the
ScoreChanges without extra copies (or alternatively make a local non-const copy
and move from it). Update the buildScoreChanges signature and any call sites
that pass an lvalue accordingly; the symbols to edit are buildScoreChanges, its
parameter type UndoableTransaction::ChangesInfo, and the moved members
changes.changedObjects, changes.changedObjectTypes,
changes.changedPropertyIdSet, and changes.changedStyleIdSet.

In `@src/engraving/editing/transaction/undostack.h`:
- Around line 136-148: Const accessor methods in UndoStack return mutable
UndoableTransaction* allowing mutation via const stacks; change their return
types to const UndoableTransaction* (for activeTransaction(), last(), prev(),
next(), lastAtIndex()) so a const UndoStack only exposes const transactions, and
ensure uses expect const pointers or provide separate non-const overloads if
callers need mutation; update any call sites that assumed mutable pointers to
cast to non-const via non-const stack accessors or to use the new overloads, and
keep references to m_transactions and canRedo() intact.

🪄 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: CHILL

Plan: Pro

Run ID: 87bcef39-bcf3-433c-a4a0-696a0da80ae0

📥 Commits

Reviewing files that changed from the base of the PR and between aa38811 and 4c4af05.

📒 Files selected for processing (129)

• src/engraving/CMakeLists.txt
• src/engraving/api/v1/elements.h
• src/engraving/api/v1/selection.cpp
• src/engraving/compat/midi/compatmidirender.cpp
• src/engraving/dom/arpeggio.cpp
• src/engraving/dom/barline.cpp
• src/engraving/dom/box.cpp
• src/engraving/dom/capo.cpp
• src/engraving/dom/capo.h
• src/engraving/dom/clef.cpp
• src/engraving/dom/durationelement.cpp
• src/engraving/dom/dynamichairpingroup.cpp
• src/engraving/dom/engravingitem.cpp
• src/engraving/dom/engravingobject.cpp
• src/engraving/dom/figuredbass.cpp
• src/engraving/dom/harmony.cpp
• src/engraving/dom/lyrics.cpp
• src/engraving/dom/masterscore.cpp
• src/engraving/dom/masterscore.h
• src/engraving/dom/pitchspelling.cpp
• src/engraving/dom/pitchspelling.h
• src/engraving/dom/playcounttext.cpp
• src/engraving/dom/score.cpp
• src/engraving/dom/score.h
• src/engraving/dom/slur.cpp
• src/engraving/dom/stafftextbase.cpp
• src/engraving/dom/stafftextbase.h
• src/engraving/dom/stringtunings.cpp
• src/engraving/dom/system.cpp
• src/engraving/dom/textbase.cpp
• src/engraving/dom/tie.cpp
• src/engraving/editing/addremoveelement.cpp
• src/engraving/editing/addremoveelement.h
• src/engraving/editing/cmd.cpp
• src/engraving/editing/cmd.h
• src/engraving/editing/edit.cpp
• src/engraving/editing/editbrackets.h
• src/engraving/editing/editchord.h
• src/engraving/editing/editclef.h
• src/engraving/editing/editenharmonicspelling.cpp
• src/engraving/editing/editenharmonicspelling.h
• src/engraving/editing/editexcerpt.h
• src/engraving/editing/editfretboarddiagram.h
• src/engraving/editing/editharppedaldiagram.h
• src/engraving/editing/editinstrumentchange.h
• src/engraving/editing/editkeysig.h
• src/engraving/editing/editmeasures.cpp
• src/engraving/editing/editmeasures.h
• src/engraving/editing/editnote.cpp
• src/engraving/editing/editnote.h
• src/engraving/editing/editpart.h
• src/engraving/editing/editproperty.cpp
• src/engraving/editing/editproperty.h
• src/engraving/editing/editscoreproperties.h
• src/engraving/editing/editsoundflag.h
• src/engraving/editing/editspanner.h
• src/engraving/editing/editstaff.h
• src/engraving/editing/editstavesharing.cpp
• src/engraving/editing/editstavesharing.h
• src/engraving/editing/editstyle.cpp
• src/engraving/editing/editstyle.h
• src/engraving/editing/editsystemlocks.cpp
• src/engraving/editing/edittie.h
• src/engraving/editing/edittremolo.h
• src/engraving/editing/exchangevoices.cpp
• src/engraving/editing/inserttime.h
• src/engraving/editing/textedit.cpp
• src/engraving/editing/textedit.h
• src/engraving/editing/transaction/undoablecommand.cpp
• src/engraving/editing/transaction/undoablecommand.h
• src/engraving/editing/transaction/undostack.cpp
• src/engraving/editing/transaction/undostack.h
• src/engraving/editing/transpose.cpp
• src/engraving/editing/undo.h
• src/engraving/playback/playbackcontext.cpp
• src/engraving/playback/playbackmodel.cpp
• src/engraving/qml/MuseScore/Engraving/devtools/engravingundostackmodel.cpp
• src/engraving/qml/MuseScore/Engraving/devtools/engravingundostackmodel.h
• src/engraving/rendering/score/arpeggiolayout.cpp
• src/engraving/rendering/score/boxlayout.cpp
• src/engraving/rendering/score/chordlayout.cpp
• src/engraving/rendering/score/harmonylayout.cpp
• src/engraving/rendering/score/layoutcontext.cpp
• src/engraving/rendering/score/layoutcontext.h
• src/engraving/rendering/score/lyricslayout.cpp
• src/engraving/rendering/score/measurenumberlayout.cpp
• src/engraving/rendering/score/restlayout.cpp
• src/engraving/rendering/score/segmentlayout.cpp
• src/engraving/rendering/score/systemheaderlayout.cpp
• src/engraving/rendering/score/tlayout.h
• src/engraving/rendering/single/singledraw.cpp
• src/engraving/rendering/single/singlelayout.cpp
• src/engraving/rw/compat/compatutils.cpp
• src/engraving/rw/read206/read206.cpp
• src/engraving/rw/read400/readcontext.cpp
• src/engraving/rw/read400/tread.cpp
• src/engraving/rw/read410/readcontext.cpp
• src/engraving/rw/read410/tread.cpp
• src/engraving/rw/read460/readcontext.cpp
• src/engraving/rw/read460/tread.cpp
• src/engraving/rw/read500/readcontext.cpp
• src/engraving/rw/read500/tread.cpp
• src/engraving/tests/box_tests.cpp
• src/engraving/tests/breath_tests.cpp
• src/engraving/tests/earlymusic_tests.cpp
• src/engraving/tests/exchangevoices_tests.cpp
• src/engraving/tests/implodeexplode_tests.cpp
• src/engraving/tests/keysig_tests.cpp
• src/engraving/tests/measure_tests.cpp
• src/engraving/tests/readwriteundoreset_tests.cpp
• src/engraving/tests/spanners_tests.cpp
• src/engraving/tests/textbase_tests.cpp
• src/engraving/tests/timesig_tests.cpp
• src/engraving/tests/tools_tests.cpp
• src/engraving/tests/transpose_tests.cpp
• src/importexport/bb/internal/bb.cpp
• src/importexport/musicxml/internal/export/exportmusicxml.cpp
• src/importexport/tabledit/internal/importtef.cpp
• src/instrumentsscene/qml/MuseScore/InstrumentsScene/internal/systemobjectslayertreeitem.cpp
• src/notation/internal/inotationundostack.h
• src/notation/internal/masternotation.cpp
• src/notation/internal/notationinteraction.cpp
• src/notation/internal/notationundostack.cpp
• src/notation/internal/notationundostack.h
• src/notationscene/qml/MuseScore/NotationScene/elementpopups/partialtiepopupmodel.cpp
• src/notationscene/qml/MuseScore/NotationScene/percussionpanel/percussionpanelmodel.cpp
• src/notationscene/widgets/editstaff.cpp
• src/palette/internal/palettecreator.cpp
• src/propertiespanel/qml/MuseScore/PropertiesPanel/emptystaves/emptystavesvisiblitysettingsmodel.cpp

💤 Files with no reviewable changes (27)

• src/engraving/dom/box.cpp
• src/engraving/dom/lyrics.cpp
• src/engraving/tests/readwriteundoreset_tests.cpp
• src/engraving/api/v1/elements.h
• src/engraving/rw/read500/readcontext.cpp
• src/engraving/dom/figuredbass.cpp
• src/engraving/compat/midi/compatmidirender.cpp
• src/engraving/rw/read460/readcontext.cpp
• src/engraving/dom/clef.cpp
• src/engraving/dom/arpeggio.cpp
• src/engraving/editing/undo.h
• src/engraving/dom/barline.cpp
• src/engraving/dom/harmony.cpp
• src/engraving/dom/textbase.cpp
• src/engraving/rw/read410/readcontext.cpp
• src/engraving/dom/dynamichairpingroup.cpp
• src/propertiespanel/qml/MuseScore/PropertiesPanel/emptystaves/emptystavesvisiblitysettingsmodel.cpp
• src/engraving/rendering/score/chordlayout.cpp
• src/engraving/dom/tie.cpp
• src/engraving/dom/pitchspelling.h
• src/notation/internal/masternotation.cpp
• src/notationscene/qml/MuseScore/NotationScene/percussionpanel/percussionpanelmodel.cpp
• src/engraving/rendering/score/boxlayout.cpp
• src/engraving/dom/system.cpp
• src/importexport/musicxml/internal/export/exportmusicxml.cpp
• src/engraving/dom/pitchspelling.cpp
• src/engraving/editing/editproperty.cpp

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/engraving/api/v1/selection.cpp (1)

139-143: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve the legacy command bridge here.

This now takes the tick-based path only for hasActiveTransaction(). In this step-1 migration, callers still using the compatibility command flow will fall back to setRange(...) and keep raw Segment* state again, so edits inside the same command can leave the range selection stale or dangling. Use a unified “recording undo state” check here, or keep the legacy active-command predicate in the condition.

Suggested fix

-    if (segEnd && m_selection->score()->undoStack()->hasActiveTransaction()) {
+    if (segEnd && (m_selection->score()->undoStack()->hasActiveTransaction()
+                   || m_selection->score()->undoStack()->hasActiveCommand())) {
         m_selection->setRangeTicks(segStart->tick(), segEnd->tick(), startStaff, endStaff);
     } else {
         m_selection->setRange(segStart, segEnd, startStaff, endStaff);
     }

🤖 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/engraving/api/v1/selection.cpp` around lines 139 - 143, The current
condition only uses hasActiveTransaction() and so calls setRangeTicks only for
that case; restore the legacy command-bridge behavior by making the condition a
unified "recording undo state" check — i.e., change the if to test
undoStack()->isRecording() (if such API exists) or OR in the legacy
command-bridge predicate (the previous compatibility check on
score()->undoStack(), e.g. undoStack()->inCommandBridge() or equivalent) so that
when the undo stack is recording or the legacy command bridge is active you call
m_selection->setRangeTicks(...), otherwise call m_selection->setRange(...).

🤖 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.

Outside diff comments:
In `@src/engraving/api/v1/selection.cpp`:
- Around line 139-143: The current condition only uses hasActiveTransaction()
and so calls setRangeTicks only for that case; restore the legacy command-bridge
behavior by making the condition a unified "recording undo state" check — i.e.,
change the if to test undoStack()->isRecording() (if such API exists) or OR in
the legacy command-bridge predicate (the previous compatibility check on
score()->undoStack(), e.g. undoStack()->inCommandBridge() or equivalent) so that
when the undo stack is recording or the legacy command bridge is active you call
m_selection->setRangeTicks(...), otherwise call m_selection->setRange(...).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2d9e8394-9937-4c3e-b27d-cbb6c8b20075

📥 Commits

Reviewing files that changed from the base of the PR and between 4c4af05 and 25eef2c.

📒 Files selected for processing (98)

• src/engraving/CMakeLists.txt
• src/engraving/api/v1/elements.h
• src/engraving/api/v1/selection.cpp
• src/engraving/compat/midi/compatmidirender.cpp
• src/engraving/dom/arpeggio.cpp
• src/engraving/dom/barline.cpp
• src/engraving/dom/box.cpp
• src/engraving/dom/clef.cpp
• src/engraving/dom/durationelement.cpp
• src/engraving/dom/dynamichairpingroup.cpp
• src/engraving/dom/engravingitem.cpp
• src/engraving/dom/engravingobject.cpp
• src/engraving/dom/figuredbass.cpp
• src/engraving/dom/harmony.cpp
• src/engraving/dom/lyrics.cpp
• src/engraving/dom/masterscore.cpp
• src/engraving/dom/masterscore.h
• src/engraving/dom/playcounttext.cpp
• src/engraving/dom/score.cpp
• src/engraving/dom/score.h
• src/engraving/dom/slur.cpp
• src/engraving/dom/system.cpp
• src/engraving/dom/textbase.cpp
• src/engraving/dom/tie.cpp
• src/engraving/editing/addremoveelement.cpp
• src/engraving/editing/addremoveelement.h
• src/engraving/editing/cmd.cpp
• src/engraving/editing/cmd.h
• src/engraving/editing/edit.cpp
• src/engraving/editing/editbrackets.h
• src/engraving/editing/editchord.h
• src/engraving/editing/editclef.h
• src/engraving/editing/editexcerpt.h
• src/engraving/editing/editfretboarddiagram.h
• src/engraving/editing/editharppedaldiagram.h
• src/engraving/editing/editinstrumentchange.h
• src/engraving/editing/editkeysig.h
• src/engraving/editing/editmeasures.cpp
• src/engraving/editing/editmeasures.h
• src/engraving/editing/editnote.cpp
• src/engraving/editing/editnote.h
• src/engraving/editing/editpart.h
• src/engraving/editing/editproperty.h
• src/engraving/editing/editscoreproperties.h
• src/engraving/editing/editsoundflag.h
• src/engraving/editing/editspanner.h
• src/engraving/editing/editstaff.h
• src/engraving/editing/editstavesharing.cpp
• src/engraving/editing/editstavesharing.h
• src/engraving/editing/editstyle.cpp
• src/engraving/editing/editstyle.h
• src/engraving/editing/editsystemlocks.cpp
• src/engraving/editing/edittie.h
• src/engraving/editing/edittremolo.h
• src/engraving/editing/exchangevoices.cpp
• src/engraving/editing/inserttime.h
• src/engraving/editing/textedit.cpp
• src/engraving/editing/textedit.h
• src/engraving/editing/transaction/undoablecommand.cpp
• src/engraving/editing/transaction/undoablecommand.h
• src/engraving/editing/transaction/undostack.cpp
• src/engraving/editing/transaction/undostack.h
• src/engraving/editing/transpose.cpp
• src/engraving/playback/playbackmodel.cpp
• src/engraving/qml/MuseScore/Engraving/devtools/engravingundostackmodel.cpp
• src/engraving/qml/MuseScore/Engraving/devtools/engravingundostackmodel.h
• src/engraving/rendering/score/boxlayout.cpp
• src/engraving/rendering/score/chordlayout.cpp
• src/engraving/rendering/score/layoutcontext.cpp
• src/engraving/rendering/score/layoutcontext.h
• src/engraving/rw/read206/read206.cpp
• src/engraving/rw/read400/readcontext.cpp
• src/engraving/rw/read400/tread.cpp
• src/engraving/rw/read410/readcontext.cpp
• src/engraving/rw/read410/tread.cpp
• src/engraving/rw/read460/readcontext.cpp
• src/engraving/rw/read500/readcontext.cpp
• src/engraving/tests/box_tests.cpp
• src/engraving/tests/breath_tests.cpp
• src/engraving/tests/earlymusic_tests.cpp
• src/engraving/tests/exchangevoices_tests.cpp
• src/engraving/tests/implodeexplode_tests.cpp
• src/engraving/tests/keysig_tests.cpp
• src/engraving/tests/measure_tests.cpp
• src/engraving/tests/readwriteundoreset_tests.cpp
• src/engraving/tests/spanners_tests.cpp
• src/engraving/tests/textbase_tests.cpp
• src/engraving/tests/timesig_tests.cpp
• src/engraving/tests/tools_tests.cpp
• src/engraving/tests/transpose_tests.cpp
• src/importexport/musicxml/internal/export/exportmusicxml.cpp
• src/instrumentsscene/qml/MuseScore/InstrumentsScene/internal/systemobjectslayertreeitem.cpp
• src/notation/internal/masternotation.cpp
• src/notation/internal/notationundostack.cpp
• src/notationscene/qml/MuseScore/NotationScene/elementpopups/partialtiepopupmodel.cpp
• src/notationscene/qml/MuseScore/NotationScene/percussionpanel/percussionpanelmodel.cpp
• src/notationscene/widgets/editstaff.cpp
• src/propertiespanel/qml/MuseScore/PropertiesPanel/emptystaves/emptystavesvisiblitysettingsmodel.cpp

💤 Files with no reviewable changes (24)

• src/engraving/dom/arpeggio.cpp
• src/engraving/dom/lyrics.cpp
• src/engraving/dom/tie.cpp
• src/engraving/dom/figuredbass.cpp
• src/engraving/tests/readwriteundoreset_tests.cpp
• src/engraving/rw/read460/readcontext.cpp
• src/engraving/rendering/score/boxlayout.cpp
• src/engraving/dom/dynamichairpingroup.cpp
• src/propertiespanel/qml/MuseScore/PropertiesPanel/emptystaves/emptystavesvisiblitysettingsmodel.cpp
• src/engraving/dom/harmony.cpp
• src/notation/internal/masternotation.cpp
• src/engraving/rendering/score/chordlayout.cpp
• src/notationscene/qml/MuseScore/NotationScene/elementpopups/partialtiepopupmodel.cpp
• src/engraving/dom/system.cpp
• src/engraving/rw/read500/readcontext.cpp
• src/engraving/dom/textbase.cpp
• src/engraving/dom/box.cpp
• src/notationscene/qml/MuseScore/NotationScene/percussionpanel/percussionpanelmodel.cpp
• src/engraving/compat/midi/compatmidirender.cpp
• src/engraving/dom/clef.cpp
• src/engraving/dom/barline.cpp
• src/engraving/api/v1/elements.h
• src/engraving/rw/read410/readcontext.cpp
• src/importexport/musicxml/internal/export/exportmusicxml.cpp

✅ Files skipped from review due to trivial changes (15)

• src/engraving/tests/box_tests.cpp
• src/engraving/dom/masterscore.cpp
• src/engraving/dom/engravingitem.cpp
• src/engraving/tests/breath_tests.cpp
• src/engraving/tests/transpose_tests.cpp
• src/engraving/dom/slur.cpp
• src/engraving/tests/exchangevoices_tests.cpp
• src/engraving/tests/textbase_tests.cpp
• src/instrumentsscene/qml/MuseScore/InstrumentsScene/internal/systemobjectslayertreeitem.cpp
• src/engraving/tests/keysig_tests.cpp
• src/engraving/dom/playcounttext.cpp
• src/engraving/tests/earlymusic_tests.cpp
• src/engraving/playback/playbackmodel.cpp
• src/engraving/tests/timesig_tests.cpp
• src/engraving/tests/measure_tests.cpp

🚧 Files skipped from review as they are similar to previous changes (50)

• src/engraving/editing/editstyle.cpp
• src/engraving/editing/editclef.h
• src/engraving/rendering/score/layoutcontext.cpp
• src/engraving/editing/editinstrumentchange.h
• src/engraving/tests/spanners_tests.cpp
• src/engraving/editing/editsystemlocks.cpp
• src/notation/internal/notationundostack.cpp
• src/engraving/editing/exchangevoices.cpp
• src/engraving/rw/read400/readcontext.cpp
• src/engraving/qml/MuseScore/Engraving/devtools/engravingundostackmodel.h
• src/engraving/editing/editharppedaldiagram.h
• src/engraving/dom/engravingobject.cpp
• src/engraving/editing/editkeysig.h
• src/engraving/editing/inserttime.h
• src/engraving/tests/implodeexplode_tests.cpp
• src/engraving/dom/durationelement.cpp
• src/engraving/editing/editsoundflag.h
• src/engraving/editing/editspanner.h
• src/engraving/editing/editmeasures.h
• src/engraving/rw/read410/tread.cpp
• src/engraving/editing/edittie.h
• src/engraving/rw/read400/tread.cpp
• src/engraving/editing/cmd.h
• src/engraving/editing/editstavesharing.h
• src/engraving/editing/editbrackets.h
• src/engraving/editing/transaction/undoablecommand.cpp
• src/engraving/editing/textedit.h
• src/engraving/editing/editmeasures.cpp
• src/engraving/editing/textedit.cpp
• src/engraving/editing/editnote.h
• src/engraving/editing/editstyle.h
• src/engraving/rendering/score/layoutcontext.h
• src/engraving/editing/editchord.h
• src/engraving/editing/editstaff.h
• src/engraving/editing/editproperty.h
• src/engraving/editing/editexcerpt.h
• src/engraving/editing/editfretboarddiagram.h
• src/engraving/dom/masterscore.h
• src/engraving/editing/addremoveelement.h
• src/engraving/rw/read206/read206.cpp
• src/notationscene/widgets/editstaff.cpp
• src/engraving/qml/MuseScore/Engraving/devtools/engravingundostackmodel.cpp
• src/engraving/editing/editnote.cpp
• src/engraving/editing/editscoreproperties.h
• src/engraving/editing/editpart.h
• src/engraving/editing/edit.cpp
• src/engraving/editing/transaction/undoablecommand.h
• src/engraving/editing/transaction/undostack.h
• src/engraving/editing/addremoveelement.cpp
• src/engraving/editing/transaction/undostack.cpp

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (1)

src/engraving/editing/transaction/undostack.cpp (1)

242-244: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

reopen() still erases the wrong state entry.

After --m_currentIndex and takeAt(m_transactions, m_currentIndex), the state that belongs to the reopened transaction is the next slot, not m_currentIndex. Erasing the current slot drops the previous committed state and leaves m_states out of sync with m_transactions.

Suggested fix

-    m_states.erase(m_states.begin() + m_currentIndex);
+    m_states.erase(m_states.begin() + m_currentIndex + 1);

🤖 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/engraving/editing/transaction/undostack.cpp` around lines 242 - 244, In
reopen() the code decrements m_currentIndex and calls m_activeTransaction =
muse::takeAt(m_transactions, m_currentIndex) but then erases m_states at
m_currentIndex, which removes the previous committed state and misaligns
m_states with m_transactions; change the erase to remove the state that belongs
to the reopened transaction (erase m_states.begin() + (m_currentIndex + 1)) so
m_states remains in sync with the transaction removed by muse::takeAt, or adjust
the ordering so the state removed matches the transaction index returned by
muse::takeAt.

🤖 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/engraving/editing/cmd.cpp`:
- Around line 457-467: MasterScore::endCmd() currently forces rollback but still
calls update(false, layoutAllParts) which triggers deletePostponed(m_cmdState)
and permanently removes objects queued via CmdState::deleteLater(); to fix,
ensure postponed deletions are either skipped or cleared when rollback is true
before calling update: in MasterScore::endCmd() detect rollback and call a new
CmdState method (e.g., cancelPostponedDeletions() or resetPostponed()) to drain
or clear m_postponedDeletions without deleting, or modify deletePostponed() to
check a rollback flag on CmdState; update references are MasterScore::endCmd(),
undoStack()->endTransaction(rollback), update(), deletePostponed(),
CmdState::m_postponedDeletions and CmdState::deleteLater() so the change
prevents permanent deletions during rolled-back transactions.

In `@src/engraving/editing/transaction/undostack.cpp`:
- Around line 215-217: The rollback path in endTransaction currently calls
m_activeTransaction->unwind(), which only calls undo() and deletes commands,
skipping the new cleanup(bool wasDone) hook and causing resource leaks for
commands like AddNoteParenthesisInfo and InsertStaff; update the rollback flow
so cleanup(false) is invoked for each command before deletion (either by
modifying UndoableTransaction::unwind to call command->cleanup(false) after
undo(), or by adding a dedicated m_activeTransaction->cleanup(false) call from
endTransaction when rollback is true) so that ownership cleanup runs for
rolled-back commands.

---

Duplicate comments:
In `@src/engraving/editing/transaction/undostack.cpp`:
- Around line 242-244: In reopen() the code decrements m_currentIndex and calls
m_activeTransaction = muse::takeAt(m_transactions, m_currentIndex) but then
erases m_states at m_currentIndex, which removes the previous committed state
and misaligns m_states with m_transactions; change the erase to remove the state
that belongs to the reopened transaction (erase m_states.begin() +
(m_currentIndex + 1)) so m_states remains in sync with the transaction removed
by muse::takeAt, or adjust the ordering so the state removed matches the
transaction index returned by muse::takeAt.

🪄 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: CHILL

Plan: Pro

Run ID: 5a28d452-9154-402b-875f-3e80e2cf5e1a

📥 Commits

Reviewing files that changed from the base of the PR and between 25eef2c and 9bee701.

📒 Files selected for processing (12)

• src/engraving/editing/addremoveelement.cpp
• src/engraving/editing/cmd.cpp
• src/engraving/editing/editchord.cpp
• src/engraving/editing/editchord.h
• src/engraving/editing/editexcerpt.cpp
• src/engraving/editing/editexcerpt.h
• src/engraving/editing/editpart.cpp
• src/engraving/editing/editstaff.cpp
• src/engraving/editing/editsystemlocks.cpp
• src/engraving/editing/transaction/undoablecommand.h
• src/engraving/editing/transaction/undostack.cpp
• src/engraving/editing/transaction/undostack.h

✅ Files skipped from review due to trivial changes (2)

• src/engraving/editing/editexcerpt.cpp
• src/engraving/editing/editpart.cpp

🚧 Files skipped from review as they are similar to previous changes (5)

• src/engraving/editing/editsystemlocks.cpp
• src/engraving/editing/transaction/undoablecommand.h
• src/engraving/editing/transaction/undostack.h
• src/engraving/editing/editchord.h
• src/engraving/editing/addremoveelement.cpp

[coderabbitai]

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 `@src/notation/internal/notationinteraction.cpp`:
- Around line 1389-1392: Reset m_editData.isHairpinDragCreatedFromDynamic after
consuming it and only call m_undoStack->mergeTransactions when there are at
least two prior states: check m_undoStack->currentStateIndex() >= 2 before
computing currentStateIndex() - 2 to avoid underflow, perform the merge, then
set m_editData.isHairpinDragCreatedFromDynamic = false so subsequent drags don't
incorrectly merge unrelated undo steps.

🪄 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: CHILL

Plan: Pro

Run ID: e21c4d3d-cafe-4e21-a776-3a5493b88615

📥 Commits

Reviewing files that changed from the base of the PR and between 3dcc172 and 6f51851.

📒 Files selected for processing (131)

• src/braille/internal/braille.cpp
• src/engraving/CMakeLists.txt
• src/engraving/api/v1/elements.h
• src/engraving/api/v1/selection.cpp
• src/engraving/compat/midi/compatmidirender.cpp
• src/engraving/dom/arpeggio.cpp
• src/engraving/dom/barline.cpp
• src/engraving/dom/box.cpp
• src/engraving/dom/capo.cpp
• src/engraving/dom/capo.h
• src/engraving/dom/clef.cpp
• src/engraving/dom/durationelement.cpp
• src/engraving/dom/dynamichairpingroup.cpp
• src/engraving/dom/engravingitem.cpp
• src/engraving/dom/engravingobject.cpp
• src/engraving/dom/figuredbass.cpp
• src/engraving/dom/harmony.cpp
• src/engraving/dom/lyrics.cpp
• src/engraving/dom/masterscore.cpp
• src/engraving/dom/masterscore.h
• src/engraving/dom/playcounttext.cpp
• src/engraving/dom/score.cpp
• src/engraving/dom/score.h
• src/engraving/dom/slur.cpp
• src/engraving/dom/stafftextbase.cpp
• src/engraving/dom/stafftextbase.h
• src/engraving/dom/stringtunings.cpp
• src/engraving/dom/system.cpp
• src/engraving/dom/textbase.cpp
• src/engraving/dom/tie.cpp
• src/engraving/editing/addremoveelement.cpp
• src/engraving/editing/addremoveelement.h
• src/engraving/editing/cmd.cpp
• src/engraving/editing/cmd.h
• src/engraving/editing/edit.cpp
• src/engraving/editing/editbrackets.h
• src/engraving/editing/editchord.cpp
• src/engraving/editing/editchord.h
• src/engraving/editing/editclef.h
• src/engraving/editing/editexcerpt.cpp
• src/engraving/editing/editexcerpt.h
• src/engraving/editing/editfretboarddiagram.h
• src/engraving/editing/editharppedaldiagram.h
• src/engraving/editing/editinstrumentchange.h
• src/engraving/editing/editkeysig.h
• src/engraving/editing/editmeasures.cpp
• src/engraving/editing/editmeasures.h
• src/engraving/editing/editnote.cpp
• src/engraving/editing/editnote.h
• src/engraving/editing/editpart.cpp
• src/engraving/editing/editpart.h
• src/engraving/editing/editproperty.cpp
• src/engraving/editing/editproperty.h
• src/engraving/editing/editscoreproperties.h
• src/engraving/editing/editsoundflag.h
• src/engraving/editing/editspanner.h
• src/engraving/editing/editstaff.cpp
• src/engraving/editing/editstaff.h
• src/engraving/editing/editstavesharing.cpp
• src/engraving/editing/editstavesharing.h
• src/engraving/editing/editstyle.cpp
• src/engraving/editing/editstyle.h
• src/engraving/editing/editsystemlocks.cpp
• src/engraving/editing/edittie.h
• src/engraving/editing/edittremolo.h
• src/engraving/editing/exchangevoices.cpp
• src/engraving/editing/inserttime.h
• src/engraving/editing/textedit.cpp
• src/engraving/editing/textedit.h
• src/engraving/editing/transaction/undoablecommand.cpp
• src/engraving/editing/transaction/undoablecommand.h
• src/engraving/editing/transaction/undostack.cpp
• src/engraving/editing/transaction/undostack.h
• src/engraving/editing/transpose.cpp
• src/engraving/editing/undo.h
• src/engraving/playback/playbackcontext.cpp
• src/engraving/playback/playbackmodel.cpp
• src/engraving/qml/MuseScore/Engraving/devtools/engravingundostackmodel.cpp
• src/engraving/qml/MuseScore/Engraving/devtools/engravingundostackmodel.h
• src/engraving/rendering/score/arpeggiolayout.cpp
• src/engraving/rendering/score/boxlayout.cpp
• src/engraving/rendering/score/chordlayout.cpp
• src/engraving/rendering/score/harmonylayout.cpp
• src/engraving/rendering/score/layoutcontext.cpp
• src/engraving/rendering/score/layoutcontext.h
• src/engraving/rendering/score/lyricslayout.cpp
• src/engraving/rendering/score/measurenumberlayout.cpp
• src/engraving/rendering/score/restlayout.cpp
• src/engraving/rendering/score/segmentlayout.cpp
• src/engraving/rendering/score/systemheaderlayout.cpp
• src/engraving/rendering/score/tlayout.h
• src/engraving/rendering/single/singledraw.cpp
• src/engraving/rendering/single/singlelayout.cpp
• src/engraving/rw/compat/compatutils.cpp
• src/engraving/rw/read206/read206.cpp
• src/engraving/rw/read400/readcontext.cpp
• src/engraving/rw/read400/tread.cpp
• src/engraving/rw/read410/readcontext.cpp
• src/engraving/rw/read410/tread.cpp
• src/engraving/rw/read460/readcontext.cpp
• src/engraving/rw/read460/tread.cpp
• src/engraving/rw/read500/readcontext.cpp
• src/engraving/rw/read500/tread.cpp
• src/engraving/tests/box_tests.cpp
• src/engraving/tests/breath_tests.cpp
• src/engraving/tests/earlymusic_tests.cpp
• src/engraving/tests/exchangevoices_tests.cpp
• src/engraving/tests/harpdiagram_tests.cpp
• src/engraving/tests/implodeexplode_tests.cpp
• src/engraving/tests/keysig_tests.cpp
• src/engraving/tests/measure_tests.cpp
• src/engraving/tests/partialtie_tests.cpp
• src/engraving/tests/readwriteundoreset_tests.cpp
• src/engraving/tests/spanners_tests.cpp
• src/engraving/tests/textbase_tests.cpp
• src/engraving/tests/timesig_tests.cpp
• src/engraving/tests/tools_tests.cpp
• src/engraving/tests/transpose_tests.cpp
• src/importexport/musicxml/internal/export/exportmusicxml.cpp
• src/importexport/tabledit/internal/importtef.cpp
• src/instrumentsscene/qml/MuseScore/InstrumentsScene/internal/systemobjectslayertreeitem.cpp
• src/notation/internal/inotationundostack.h
• src/notation/internal/masternotation.cpp
• src/notation/internal/notationinteraction.cpp
• src/notation/internal/notationundostack.cpp
• src/notation/internal/notationundostack.h
• src/notationscene/qml/MuseScore/NotationScene/elementpopups/partialtiepopupmodel.cpp
• src/notationscene/qml/MuseScore/NotationScene/percussionpanel/percussionpanelmodel.cpp
• src/notationscene/widgets/editstaff.cpp
• src/palette/internal/palettecreator.cpp
• src/propertiespanel/qml/MuseScore/PropertiesPanel/emptystaves/emptystavesvisiblitysettingsmodel.cpp

💤 Files with no reviewable changes (26)

• src/engraving/compat/midi/compatmidirender.cpp
• src/notationscene/qml/MuseScore/NotationScene/percussionpanel/percussionpanelmodel.cpp
• src/engraving/dom/arpeggio.cpp
• src/engraving/dom/tie.cpp
• src/engraving/dom/clef.cpp
• src/engraving/dom/box.cpp
• src/engraving/rw/read460/readcontext.cpp
• src/engraving/rendering/score/chordlayout.cpp
• src/engraving/rw/read500/readcontext.cpp
• src/propertiespanel/qml/MuseScore/PropertiesPanel/emptystaves/emptystavesvisiblitysettingsmodel.cpp
• src/engraving/dom/figuredbass.cpp
• src/engraving/tests/harpdiagram_tests.cpp
• src/engraving/dom/lyrics.cpp
• src/engraving/tests/readwriteundoreset_tests.cpp
• src/engraving/rendering/score/boxlayout.cpp
• src/engraving/api/v1/elements.h
• src/engraving/editing/undo.h
• src/importexport/musicxml/internal/export/exportmusicxml.cpp
• src/engraving/editing/editproperty.cpp
• src/engraving/dom/textbase.cpp
• src/engraving/dom/system.cpp
• src/engraving/dom/barline.cpp
• src/engraving/dom/dynamichairpingroup.cpp
• src/engraving/dom/harmony.cpp
• src/engraving/rw/read410/readcontext.cpp
• src/notation/internal/masternotation.cpp

✅ Files skipped from review due to trivial changes (29)

• src/engraving/rendering/single/singlelayout.cpp
• src/engraving/rendering/score/segmentlayout.cpp
• src/engraving/tests/transpose_tests.cpp
• src/engraving/dom/masterscore.cpp
• src/engraving/rendering/score/arpeggiolayout.cpp
• src/engraving/tests/timesig_tests.cpp
• src/engraving/rendering/score/restlayout.cpp
• src/engraving/tests/box_tests.cpp
• src/engraving/tests/earlymusic_tests.cpp
• src/palette/internal/palettecreator.cpp
• src/engraving/tests/textbase_tests.cpp
• src/engraving/dom/stringtunings.cpp
• src/engraving/rendering/score/tlayout.h
• src/engraving/playback/playbackmodel.cpp
• src/instrumentsscene/qml/MuseScore/InstrumentsScene/internal/systemobjectslayertreeitem.cpp
• src/engraving/editing/editexcerpt.cpp
• src/engraving/tests/breath_tests.cpp
• src/engraving/rendering/score/harmonylayout.cpp
• src/engraving/rendering/score/measurenumberlayout.cpp
• src/engraving/tests/implodeexplode_tests.cpp
• src/engraving/dom/slur.cpp
• src/engraving/rendering/score/systemheaderlayout.cpp
• src/engraving/rendering/score/lyricslayout.cpp
• src/importexport/tabledit/internal/importtef.cpp
• src/engraving/dom/engravingobject.cpp
• src/engraving/rw/read460/tread.cpp
• src/engraving/rw/read500/tread.cpp
• src/engraving/rw/compat/compatutils.cpp
• src/engraving/tests/measure_tests.cpp

🚧 Files skipped from review as they are similar to previous changes (70)

• src/engraving/tests/keysig_tests.cpp
• src/engraving/tests/spanners_tests.cpp
• src/engraving/editing/editstyle.cpp
• src/notationscene/widgets/editstaff.cpp
• src/engraving/editing/editnote.cpp
• src/engraving/editing/editsoundflag.h
• src/engraving/dom/playcounttext.cpp
• src/engraving/editing/exchangevoices.cpp
• src/engraving/tests/tools_tests.cpp
• src/engraving/rendering/single/singledraw.cpp
• src/engraving/editing/editclef.h
• src/engraving/editing/editinstrumentchange.h
• src/engraving/playback/playbackcontext.cpp
• src/engraving/tests/exchangevoices_tests.cpp
• src/engraving/rw/read400/tread.cpp
• src/engraving/rendering/score/layoutcontext.h
• src/engraving/editing/editproperty.h
• src/engraving/editing/inserttime.h
• src/engraving/editing/editscoreproperties.h
• src/engraving/editing/transpose.cpp
• src/notation/internal/notationundostack.cpp
• src/engraving/editing/editstaff.cpp
• src/engraving/dom/engravingitem.cpp
• src/engraving/editing/editbrackets.h
• src/engraving/dom/durationelement.cpp
• src/engraving/qml/MuseScore/Engraving/devtools/engravingundostackmodel.cpp
• src/engraving/editing/editkeysig.h
• src/engraving/editing/editmeasures.h
• src/engraving/qml/MuseScore/Engraving/devtools/engravingundostackmodel.h
• src/engraving/CMakeLists.txt
• src/engraving/api/v1/selection.cpp
• src/engraving/editing/editsystemlocks.cpp
• src/engraving/dom/score.cpp
• src/engraving/dom/stafftextbase.h
• src/engraving/editing/editharppedaldiagram.h
• src/engraving/rw/read400/readcontext.cpp
• src/engraving/editing/editstyle.h
• src/engraving/editing/editspanner.h
• src/engraving/editing/editpart.h
• src/engraving/rendering/score/layoutcontext.cpp
• src/engraving/editing/edittremolo.h
• src/engraving/editing/editmeasures.cpp
• src/engraving/editing/editnote.h
• src/notationscene/qml/MuseScore/NotationScene/elementpopups/partialtiepopupmodel.cpp
• src/engraving/rw/read410/tread.cpp
• src/engraving/editing/edittie.h
• src/notation/internal/inotationundostack.h
• src/engraving/editing/cmd.h
• src/engraving/editing/transaction/undoablecommand.cpp
• src/engraving/dom/masterscore.h
• src/engraving/editing/editexcerpt.h
• src/engraving/editing/editfretboarddiagram.h
• src/engraving/rw/read206/read206.cpp
• src/engraving/editing/transaction/undoablecommand.h
• src/engraving/dom/stafftextbase.cpp
• src/engraving/tests/partialtie_tests.cpp
• src/engraving/editing/editstaff.h
• src/engraving/editing/editchord.h
• src/engraving/dom/capo.cpp
• src/braille/internal/braille.cpp
• src/engraving/editing/textedit.cpp
• src/engraving/editing/editpart.cpp
• src/engraving/editing/edit.cpp
• src/engraving/editing/textedit.h
• src/engraving/dom/score.h
• src/engraving/editing/addremoveelement.cpp
• src/engraving/editing/transaction/undostack.h
• src/engraving/editing/cmd.cpp
• src/engraving/editing/addremoveelement.h
• src/notation/internal/notationundostack.h

[mathesoncalum]

Hey @cbjeukendrup! We're aiming to get these reviewed next week - just a bit busy right now.

Could you elaborate a bit on what you mean by "making this system impossible to use unsafely"? Like what was the issue before and how have you addressed it (knowing this will help me review quicker).

Cheers

[cbjeukendrup]

Actually, that part has by now been moved into #33785, which contains a big code comment explaining the problem and solution.

This PR is now just some preparatory cleanups, primarily the following:

• splitting the big undo.h file
• renaming awkwardly or confusingly named symbols
• moving pieces of logic from Score into MasterScore. Strictly speaking, this might change the behaviour slightly, but that should be fine.
• introducing some sanity checks in the undoRedo method and fixing the issues revealed by that
• addressing some (only tangentially related) CodeRabbit comments

[cbjeukendrup]

Added one more tiny commit