fix(editor): restore audio-only project creation - #51
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Review pass (Claude Code). #51's own logic is correct:
|
|
Heads-up for integration: this create-modal fix is superseded by #45 (the Create New Arrangement roster redesign), which already supports from-scratch / audio-only creation — its gate needs only a title + at least one instrument, with audio optional, and the backend writes an audio-less draft (a superset of what this PR restores). Plan: at merge, resolve the create-modal region in #45's favor; no need to land this create-modal change on its own. Left open for now only because #52–#61 stack on this branch. |
Signed-off-by: ChrisBeWithYou <chris@rifflarr.local>
ac32314 to
fc43f6c
Compare
da6d576 to
3f66bec
Compare
…provenance (P2-5) (#276) * feat(editor): authored marks I — hold/fermata bars, meter groupings, provenance (P2-5) A sparse authored-intent layer over the beat grid, for what the grid cannot express. S.beats stays the single executable truth; marks are MEASURE-keyed metadata (never fields on beat entries), swapped immutably, one undoable command per edit. - src/tempo-marks.js: validation/upsert/remove/remap pures (grouping must sum to the numerator; provenance enum confirmed/detected/ suggested/imported/carried; junk degrades, never throws), the TempoMarkCmd, and the right-click verbs (hand-set marks record provenance: confirmed). - Hold/fermata: excluded from BPM stats (_tempoMeasureBpmsPure skip set — an 8s fermata bar no longer reads as a bogus 30 BPM), from the flatten-choice variability test, and from the suggest march (the fermata's span is carried at grid position, provenance-style 'carried', never window-snapped — onsets inside a hold are the sustained chord, not a pulse). The marker lane shows ONE amber hold chip instead of the spurious drop + re-entry BPM chips. - Meter grouping: 7/8 felt as 2+2+3 vs 3+2+2 — one authored chip supersedes the derived bare label. (The click/ruler/comb consumers are the P2-6 follow-up.) - Chips: authored marks render outlined (solid = confirmed, dashed = machine provenance). - Topology: _tempoRenumberMeasures returns the old→new measure map; insert/delete barline thread a marks remap through TempoGridCmd snapshots so undo restores beats + marks together; a deleted bar's marks drop honestly. - Persistence: the editor_tempo_marks manifest extension key, exactly the audio_shift contract (absent = older client leaves persisted marks alone; empty = removes the key; garbage gets no authority to erase; invalid entries dropped, never raised on). Spec issue #51 is ON HOLD — this shape is the prototype the spec proposal draws from. Tests: tempo_marks.test.mjs (11 cases, fails on main) pins stats exclusion, chip suppression/supersession, validation, remap, command round-trip, march carry; test_tempo_marks.py (5) pins the backend absent/empty/garbage contract + byte-stable round-trip. Live-verified on :8001 (AC/DC pak): verbs, chips, exact undo/redo, save → manifest key on disk → reload restores; clearing marks removes the key. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q * fix(editor): strict tempo-mark integer validation + atomic time-sig/grouping updates (review #276 items 2-3) Item 2: _coerce_tempo_marks used bare int(), which truncated fractions (measure 3.9 silently became 3), accepted bools (an int subclass) and numeric strings, and CRASHED on ±inf (OverflowError is not a ValueError). New _exact_int validator: exact finite integers only — int or integral-valued float (3.0, for float-izing JSON encoders); everything else drops the entry. Applied to measure, meter num/den, and every grouping entry; named for reuse by stacked descendants. Item 3: _tempoSetBeatsPerMeasure / _tempoSetTimeSignature changed the grid but left the bar's authored meter mark, so 7/8 (2+2+3) -> 4/4 displayed a seven-slot accent map on a four-beat bar. The edit now rides the existing TempoGridCmd marks snapshot as ONE undoable step: a grouping that still sums to the new numerator is kept and retagged (den-only changes); anything stale is CLEARED, never guessed. Undo restores beats and marks together, by reference. Both regression suites fail on the pre-fix code (6 pytest failures incl. the inf crash; 4 mjs failures incl. the surviving stale mark). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q * fix(editor): reconcile groupings on barline insert/delete + reset marks on import (review #276) Two authored-marks bugs on stateful integration paths: 1. Inserting/deleting a sync point splits/merges a bar UNDER an authored meter grouping but only REMAPPED the mark — it never reconciled the grouping against the bar's new numerator. A [2,2] mark stranded on a bar the split cut to 2 beats rendered "2/4 (2+2)" and fed a four-slot accent map to consumers: the exact stale-accent lie the time-sig reconcile (item 3) prevents, reached by the topology path. Fixed at the renumber seam: _tempoRemapMarksForRenumber now reconciles groupings against newBeats in the SAME TempoGridCmd, so undo restores both sides. 2. The create/import loader (editorApplyCreateResult) reset the grid but not S.tempoMarks, so a previous song's holds/groupings bled onto a freshly imported chart and persisted on its next save. Reset at the same boundary as the grid, sanitized to match loadCDLC. Regression tests fail pre-fix: - tempo_marks: insert-split drops stale grouping (same undo); delete-merge drops it; a still-honest grouping is kept (no spurious drop). - import_create_teardown: an import resets authored marks. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(tempo-marks): move comment above the TempoMarkCmd swap test it describes (CodeRabbit) * fix(editor): topology commands carry marks through renumbering (time-anchored remap) Product rule: marks follow the MUSIC. A mark rides its old bar's downbeat time into whichever new bar contains that moment (_marksRemapByTimePure); drops honestly when the moment leaves the grid or lands on a bar < 1; same-kind collisions resolve by provenance (authored beats machine), then earlier old bar. Threaded via _tempoRemapMarksByTime (null on identity) onto every audited TempoGridCmd call site that renumbers or rebuilds without a surviving-downbeat map: - tempo.js: pickup, tempo zones (refined), tempo zones (single tempo), segment-first rough map, heal uneven beats, halve-range, double-range, multi-delete barlines - main.js: flatten (rebuild-grid-only branch) - import.js: Apply MIDI tempo map The insert/delete-single and timesig paths keep their existing remap/ reconcile threading; 'mark barline' appends at the end and needs none. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q * fix(editor): octave-fix grid rescue carries marks through the rebuild editorZonesOctaveFix re-lays the whole grid but exec'd its TempoGridCmd with no .marks, so authored holds/groupings went stale (pinned to the wrong bar, not restored on undo) — the one topology path the marks-remap fix missed. Wire _tempoRemapMarksByTime like every sibling rebuild verb. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: ChrisBeWithYou <chris@rifflarr.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: byrongamatos <xasiklas@gmail.com>
2 participants
Summary
Testing