fix(bend): keep scalar bn and bnv curve consistent on Peak edits (Codex follow-up) - #15
Merged
Merged
Conversation
Post-merge Codex review of editor#13 found the scalar peak (bn) and the per-note bend curve (bnv) could drift apart, since renderers/graders treat bnv as authoritative: - New pure helper rescaleBendCurveToPeak (in the @pure:bend-shape block) rescales a curve's shape to a target peak, returning null when the curve is empty, all-zero, or the target is below bnv's 0.1 precision (rounds wholly to 0) — i.e. whenever it can't faithfully carry the peak. - Bend modal Peak input now rescales the curve to the new peak, or clears it (Peak 0, or unscalable) so the OK reconcile keeps the typed Peak instead of deriving bn=0 from a leftover zero curve. - Bend modal OK still derives bn from the curve peak (curve wins when present). - Inspector editorInspectorSetTech('bend') rescales the curve and snaps bn to the curve's actual rounded peak, so a non-0.1 input (e.g. 0.25) can't leave bn and the curve disagreeing. Inspector edits intentionally skip undo (documented PR3b convention), so the bend path stays a direct mutation like its siblings rather than a SetBendShapeCmd. Codex-reviewed across 3 rounds: clean. +5 node tests for rescaleBendCurveToPeak (rescale up/down, 0.1 rounding, unscalable->null, 0.04->null / 0.05->0.1 boundary). 14/14 bend-shape tests pass. Part of got-feedBack/feedBack#334. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Follow-up to #13. A post-merge Codex review found that the scalar bend peak (
bn) and the per-note bend curve (bnv) could drift apart after an edit — and renderers/graders treatbnvas authoritative, so a stalebnis a real inconsistency.Fix
rescaleBendCurveToPeak(in the@pure:bend-shapeblock, so it's node-tested by extraction): rescales a curve's shape to a target peak. Returnsnullwhen the curve is empty, all-zero, or the target is belowbnv's 0.1 precision (rounds wholly to 0) — i.e. whenever the curve can't faithfully carry the peak.bn=0from a leftover zero curve.bnfrom the curve peak (curve wins when one is present).editorInspectorSetTech('bend')→ rescales the curve and snapsbnto the curve's actual rounded peak, so a non-0.1 input (e.g.0.25) can't leavebnand the curve disagreeing.Inspector edits intentionally skip undo (documented
PR3bconvention,screen.js:3783), so the bend path stays a direct mutation like its siblings rather than aSetBendShapeCmd.Review
Codex-reviewed across 3 rounds (caught the non-0.1 mismatch and the sub-0.1 all-zero-curve edge along the way): clean, no findings.
Tests
+5 node cases for
rescaleBendCurveToPeak(rescale up/down, 0.1 rounding, unscalable→null, the0.04→null/0.05→0.1precision boundary). 14/14 bend-shape tests pass;node --checkclean.Part of got-feedBack/feedBack#334.
🤖 Generated with Claude Code