refactor(editor): extract the note/chord pure tier to src/notes.js (R2, step 6)

[byrongamatos]

Step 6 of the editor's ES-module split (R2). main.js 20,763 → 20,601.

What moved

src/notes.js (176 lines) — the note/chord data model, pure tier. Reads S; no DOM, no undo history. Graph stays acyclic: notes → state.

• accessors: notes(), chords() — 204 and 35 call sites, none of which changed
• chord-aware sustain resizing: _resizeTargetIndicesPure, _maxSustainBeforeCollisionPure, _resizeSustainsForDeltaPure
• bend-curve authoring: bendPresetCurve, sanitizeBendCurve, rescaleBendCurveToPeak, BEND_INTENTS
• teaching marks: FRET_FINGER_OPTIONS, nextUnusedStrumGroup

main.js's entire diff is the deletions plus the import block (checked by reconstructing it programmatically).

Tests: four more source-slicers die

• chord_resize, bend_shape, teaching_marks → plain real-import tests.
• roll_edge_resize → hybrid: still slices the edit-history block and the ResizeSustainCmd/ResizeSustainGroupCmd classes that stay in main.js, but imports the resize arithmetic. Those commands call notes() through an injected sandbox param already, so they were unaffected.
• New tests/notes.test.mjs covers what the pure-fn suites never touched: the accessors' empty-arrangement guard (200+ call sites iterate the result directly, including on the entry screen with nothing loaded) and that they return the live array, not a copy.

Why chord-relink and reconstructChords did NOT come along

They're the obvious next thing in that section, and I left them deliberately.

flattenChords needs _normChordFn from inside the @pure:chord-relink block, and reconstructChords is entangled with S.history. But the real blocker is the tests: chord_relink, handshape_authoring and suggest_position_wiring concatenate that block's source text into a new Function sandbox and drive it against a fabricated S. A real import would close over the actual S from state.js and quietly bypass their fixtures — the tests would still pass while testing the wrong object.

So those three need a genuine rework (drive the real S, the way lanes.test.mjs and notes.test.mjs do). That's a step of its own, not a rider on a mechanical move. Shipping it here would have meant a 600-line move plus three non-trivial test rewrites in one reviewable unit.

Verification

• node --test 85/85, pytest 248/248.
• No unused import. No shadowing that changes behaviour: notes and chords do appear as parameter names elsewhere in main.js (e.g. dropOrphanedHandshapes(handshapes, chords, notes)) — they shadowed the function declaration before and shadow the import now, identically.
• Served from local uvicorn on core@main (R0): src/notes.js 200 as text/javascript.
• All four headless Chromium harnesses green: draw path, state round-trip, hit test, resize.
• Codex preflight: NO ISSUES.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Improved note and chord editing support, including better sustain resizing behavior for chorded notes.
  • Added more reliable bend-curve handling and teaching-mark options in the editor.

• Bug Fixes

  • Active note and chord data now resolves more consistently from the current arrangement.
  • Added coverage to prevent regressions around empty arrangements and live-updating note/chord lists.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9254c189-58a3-45ab-8743-527143ad1dc6

📥 Commits

Reviewing files that changed from the base of the PR and between 87971d0 and a658772.

📒 Files selected for processing (8)

• CHANGELOG.md
• src/main.js
• src/notes.js
• tests/bend_shape.test.mjs
• tests/chord_resize.test.mjs
• tests/notes.test.mjs
• tests/roll_edge_resize.test.mjs
• tests/teaching_marks.test.mjs

---

📝 Walkthrough

Walkthrough

Pure note/chord logic (accessors, chord-resize sustain arithmetic, bend-curve utilities, teaching-mark helpers) is extracted from src/main.js into a new src/notes.js module. main.js now imports these from notes.js. Related test files migrate from CommonJS eval-based extraction to direct ESM imports; a new accessor test file is added; CHANGELOG.md documents the migration step.

Changes

Note/chord pure tier migration

Layer / File(s)	Summary
notes()/chords() active-arrangement accessors src/notes.js, src/main.js, tests/notes.test.mjs	Adds notes()/chords() accessors to notes.js backed by state S, removes their local definitions and wires the import in main.js, and adds a new test verifying empty-arrangement guards and live-array (not copy) behavior.
Chord-aware sustain resize helpers src/notes.js, src/main.js, tests/chord_resize.test.mjs, tests/roll_edge_resize.test.mjs	Adds _resizeTargetIndicesPure, _maxSustainBeforeCollisionPure, _resizeSustainsForDeltaPure to notes.js, removes them from main.js, and updates both test files to import directly from notes.js instead of extracting @pure: blocks.
Bend-curve authoring and persistence utilities src/notes.js, src/main.js, tests/bend_shape.test.mjs	Adds BEND_INTENTS, bendPresetCurve, sanitizeBendCurve, rescaleBendCurveToPeak to notes.js, removes them from main.js, and updates the test to import directly from notes.js.
Teaching-mark authoring helpers and changelog src/notes.js, tests/teaching_marks.test.mjs, CHANGELOG.md	Adds FRET_FINGER_OPTIONS and nextUnusedStrumGroup to notes.js, updates the test to import directly, and adds a changelog entry describing the migration.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Test as notes.test.mjs
  participant NotesModule as notes.js
  participant State as S (global state)
  Test->>NotesModule: call notes()
  NotesModule->>State: read S.arrangements[S.currentArr]
  State-->>NotesModule: active arrangement notes array (live reference)
  NotesModule-->>Test: return live array
  Test->>Test: mutate returned array
  Test->>State: verify underlying arrangement updated

Loading

🚥 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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: extracting the note/chord pure tier into src/notes.js for R2 step 6.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/es-module-split-notes

Warning

Tools execution failed with the following error:

Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error)

---

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

[Copilot]

Pull request overview

This PR continues the editor’s ES-module split by extracting the note/chord “pure tier” (accessors + arithmetic helpers) from src/main.js into a new leaf module src/notes.js, and updates Node-based tests to import the real module instead of slicing @pure: blocks out of main.js.

Changes:

• Added src/notes.js exporting arrangement accessors (notes(), chords()) and pure helpers for chord-aware sustain resizing, bend-curve authoring, and teaching marks.
• Updated multiple test suites to real-import from src/notes.js (with roll_edge_resize remaining a hybrid that still slices undo-command code from main.js).
• Documented the refactor and test migration in CHANGELOG.md, and removed the extracted code blocks from src/main.js while adding a corresponding import block.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
tests/teaching_marks.test.mjs	Switches from source-sliced @pure: eval to real imports from src/notes.js.
tests/roll_edge_resize.test.mjs	Keeps slicing undo-command code from main.js, but imports resize arithmetic from src/notes.js.
tests/notes.test.mjs	New tests covering notes()/chords() empty-arrangement guard and live-array identity behavior.
tests/chord_resize.test.mjs	Switches to real imports from src/notes.js for chord-resize arithmetic tests.
tests/bend_shape.test.mjs	Switches to real imports from src/notes.js for bend-curve helper tests.
src/notes.js	New module containing the extracted note/chord accessors and pure helper logic.
src/main.js	Removes extracted pure-tier code and imports the same APIs from src/notes.js.
CHANGELOG.md	Adds an Unreleased entry describing step 6 (notes/chords pure tier extraction and test updates).

Comments suppressed due to low confidence (3)

tests/teaching_marks.test.mjs:5

• The header comment is now inaccurate: the helpers are imported from src/notes.js (no source slicing/eval), but the comment still references src/main.js and evaluating an extracted @pure block. Update the header to match the new test approach.
  tests/bend_shape.test.mjs:5
• The header comment is now inaccurate: this test imports helpers from src/notes.js, but the comment still refers to src/main.js and evaluating an extracted block. Update the header to reflect the real-import setup.
  tests/chord_resize.test.mjs:4
• The header comment still says these helpers are for src/main.js, but the suite now imports them from src/notes.js. Update the comment to avoid confusion.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.