Fix arrange order under/overflow when pushing to the back/front

[ajuncosa]

Resolves: #27771

The To front and To back buttons were showing an apparently random behaviour.

Multiple patterns were mentioned in the discussion of #27771 . However, I was not always able to reproduce those specific cases. Instead, the cause seemed to be an underflow or overflow of the arrange order when elements of type TimeTickAnchor (which has a default Z of -INT_MAX) or SoundFlag (with a Z of INT_MAX), respectively, were present in the score. This may explain the apparent randomness of the problem.

I have added:

• boundary checks to avoid under/overflows when updating the Z order of the elements, and
• a condition to keep elements from being pushed to -INT_MAX when pushing to the back, or to INT_MAX when pushing to the front, so that there will be a larger range of movement in the Z coordinate

This should fix both the current issue as well as avoid potential underflows/overflows in case the user rearranges the order of the items (through Forwards/Backwards/To front/To back) so many times that they cause the variable to overflow again.

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[coderabbitai]

📝 Walkthrough

Walkthrough

Direct z() ± REARRANGE_ORDER_STEP updates were replaced with guarded computations that clamp results to avoid integer underflow/overflow. pushBackwardsInOrder and pushForwardsInOrder now use those clamped adjustments. pushToBackInOrder and pushToFrontInOrder use custom std::min_element/std::max_element comparators that ignore sentinel Z extremes (-INT_MAX for TimeTickAnchor, INT_MAX for SoundFlag) and perform clamped arithmetic instead of unbounded ± REARRANGE_ORDER_STEP.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: fixing underflow/overflow issues in arrange order operations when pushing elements to back/front.
Description check	✅ Passed	The description covers the issue, root cause (Z-order underflow/overflow with sentinel values), and implemented solutions with boundary checks and sentinel prevention.
Linked Issues check	✅ Passed	The PR directly addresses #27771 by fixing the erratic behavior of To front/To back operations through boundary checks and sentinel value handling to prevent underflow/overflow.
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing arrange order under/overflow in appearance settings model, directly addressing the linked issue without unrelated modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@src/inspector/qml/MuseScore/Inspector/general/appearance/appearancesettingsmodel.cpp`:
- Around line 213-215: The comparisons that compute newZ risk producing sentinel
INT_MAX/ -INT_MAX because they use exclusive bounds; update the checks around
elementBehind->z() (elementBehindZ) and analogous code that sets
m_arrangeOrder->setValue(...) so they use inclusive bounds (change '<' to '<='
and '>' to '>=') when comparing against INT_MAX - REARRANGE_ORDER_STEP and
-INT_MAX + REARRANGE_ORDER_STEP, ensuring subtracting/adding
REARRANGE_ORDER_STEP cannot yield the sentinel values; apply the same
inclusive-bound fix for all occurrences involving elementBehindZ, newZ,
REARRANGE_ORDER_STEP and m_arrangeOrder->setValue (the blocks at the referenced
lines and the similar blocks at 233-235, 257-259, 282-284).

🪄 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

Run ID: 5ceaab19-af0a-40fe-a837-db4c34040865

📥 Commits

Reviewing files that changed from the base of the PR and between be0b77f and 7ffde7e.

📒 Files selected for processing (1)

• src/inspector/qml/MuseScore/Inspector/general/appearance/appearancesettingsmodel.cpp

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@src/inspector/qml/MuseScore/Inspector/general/appearance/appearancesettingsmodel.cpp`:
- Around line 266-277: The comparator passed to std::max_element violates strict
weak ordering when both elements have z()==INT_MAX; fix the lambda in the
max_element call so it handles the INT_MAX case symmetrically: if both
e1->z()==INT_MAX and e2->z()==INT_MAX return false, otherwise if
e1->z()==INT_MAX return true (treat INT_MAX as "less" to exclude them from being
chosen), else if e2->z()==INT_MAX return false, then fall back to
elementLessThan(e1, e2); update the lambda used around EngravingItem* maxElement
and its calls to z() and elementLessThan accordingly.

🪄 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

Run ID: bb7746b4-df3f-4a8b-92a6-482e27599b3d

📥 Commits

Reviewing files that changed from the base of the PR and between 7ffde7e and d514dd0.

📒 Files selected for processing (1)

• src/inspector/qml/MuseScore/Inspector/general/appearance/appearancesettingsmodel.cpp

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@src/inspector/qml/MuseScore/Inspector/general/appearance/appearancesettingsmodel.cpp`:
- Around line 213-215: The step-wise reorder code copies sentinel Z values
(INT_MAX/ -INT_MAX) from adjacent items into normal elements; modify the logic
around elementBehind->z() (and the analogous forward case) to detect sentinel
values and avoid copying them — e.g., if elementBehindZ == -INT_MAX (or ==
INT_MAX in the forward path) compute newZ as a safe offset away from the
sentinel (such as -INT_MAX + REARRANGE_ORDER_STEP or INT_MAX -
REARRANGE_ORDER_STEP) or otherwise pick a clamped value that is not a sentinel,
then call m_arrangeOrder->setValue(newZ); ensure the resulting newZ is never
exactly INT_MAX or -INT_MAX.

🪄 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

Run ID: 5811a769-877c-44e9-ba7f-4c1148947667

📥 Commits

Reviewing files that changed from the base of the PR and between d514dd0 and 2f1ea29.

📒 Files selected for processing (1)

• src/inspector/qml/MuseScore/Inspector/general/appearance/appearancesettingsmodel.cpp