refactor(editor): collapse the per-module hooks into src/host.js (R2, step 17)

[byrongamatos]

A small consolidation that pays for the next lift.

Why

history.js, drum.js and annotation-lanes.js each grew a setXHooks() for the same reason: they need a few main.js symbols that cannot be imported back without closing a cycle. By the fourth module the same four callbacks — draw, hideContextMenu, snapTime, editorPromptText — were being threaded through three separate hook objects. The next extraction (the note command classes) needs nine.

So: one shared host object, wired once by main.js. A new module imports host and calls host.draw(). No new plumbing, no fourth setter.

// before — three objects, three setters, the same callbacks in each
setHistoryHooks({ ensureArr: _historyEnsureArr, draw: _drawLive, updateStatus });
setDrumHooks({ draw: _drawLive, drawWaveform, updateArrangementSelector });
setLaneHooks({ draw: _drawLive, hideContextMenu, snapTime, _editorPromptText });

// after
setHostHooks({
    draw: (...args) => draw(...args),
    drawWaveform, updateStatus, updateArrangementSelector,
    hideContextMenu, snapTime,
    editorPromptText: _editorPromptText,
    ensureArr: _historyEnsureArr,
});

No behaviour change. Every former _hooks.X() call site resolves to the same callback — Codex verified the mapping one-for-one, including the two renamed keys (_editorPromptText → editorPromptText, and ensureArr).

The trap, documented where it will be read

The draw thunk stays, and host.js’s header now carries the reason. draw is reassigned near the bottom of main.js to a wrapper that refreshes seven toolbar buttons; passing the bare identifier captures the original function and the refreshes silently stop. That is what shipped in #165/#166 and took a Codex review to find. The header says plainly: pass a thunk, and check grep -n ^\s*<name> = before wiring anything.

The inert defaults are type-honest rather than uniformly no-op — snapTime is the identity, editorPromptText resolves to null (a cancelled prompt) — so a module imported under node with no host wired degrades instead of crashing. That is precisely how the unit tests exercise them, and precisely why the unit tests cannot see the wiring.

Verification

Comment out the single setHostHooks(...) call: all 88 unit tests still pass, and verify_history.py, verify_drum.py and verify_lanes.py all fail. One call site now guards all three modules.

node --test 88/88 · pytest 248/248 · npm run lint 0 errors (9 warnings) · Codex clean · all 12 headless harnesses pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Refactor

  • Consolidated major UI callbacks (redraw, status updates, context-menu actions, time snapping, prompt text, and arrangement selector updates) behind a single shared wiring path used across editors and history.
  • Updated startup wiring to use shared host callbacks, keeping interactive updates consistent.

• Bug Fixes

  • Improved reliability of redraws after state changes and undo/redo.
  • Ensured context-menu interactions trigger the expected hide/close and visual refresh.

• Tests

  • Updated cross-editor undo and drum undo/velocity test environments to use the shared host wiring.

[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: 6b0fa611-9a97-4609-b31e-e514237387e0

📥 Commits

Reviewing files that changed from the base of the PR and between c018002 and f906b57.

📒 Files selected for processing (3)

• CHANGELOG.md
• src/host.js
• tests/drum_undo.test.mjs

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

• tests/drum_undo.test.mjs
• CHANGELOG.md

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

• src/host.js

---

📝 Walkthrough

Walkthrough

A new shared host module replaces per-module hook setters across main wiring, history, drum, and annotation lane code. Tests now install callbacks through setHostHooks, and the changelog records the wiring consolidation and related wording updates.

Changes

Shared Host Object Consolidation

Layer / File(s)	Summary
Host object contract and setter src/host.js	Defines the shared host object with default callbacks and setHostHooks(hooks), alongside module documentation for the shared wiring model.
main.js wiring switch to setHostHooks src/main.js	Replaces separate hook registrations with one setHostHooks call that wires draw and other UI/runtime callbacks through the shared host object.
history.js and drum.js migrated to host src/history.js, src/drum.js	Removes setHistoryHooks and setDrumHooks; undo/redo, drum rendering, and drum rollback paths now call host.* directly.
annotation-lanes.js handlers migrated to host src/annotation-lanes.js	Removes setLaneHooks; tone, anchor, and handshape mouse and context-menu handlers now call host.draw, host.snapTime, host.hideContextMenu, and host.editorPromptText.
Test harnesses and changelog updated for host wiring tests/_history_env.mjs, tests/cross_arr_undo.test.mjs, tests/drum_undo.test.mjs, tests/drum_velocity.test.mjs, CHANGELOG.md	Test hook wiring switches to setHostHooks, and the changelog text is updated to describe the shared host object and live draw binding.

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

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main refactor: consolidating per-module hooks into a shared src/host.js object.
Docstring Coverage	✅ Passed	Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.
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/r2-step17-host

---

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

[Copilot]

Pull request overview

This PR consolidates the per-module “cycle-break” hook setters (setHistoryHooks, setDrumHooks, setLaneHooks) into a single shared host object (src/host.js) that is wired once by src/main.js, allowing extracted modules to call host.* without introducing new plumbing (and avoiding import cycles).

Changes:

• Introduces src/host.js with a shared host callback object and setHostHooks() wiring function.
• Refactors src/history.js, src/drum.js, and src/annotation-lanes.js to read callbacks from host instead of per-module hook objects/setters.
• Updates src/main.js and affected unit tests to wire callbacks via setHostHooks, and documents the change in CHANGELOG.md.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/drum_velocity.test.mjs	Switches test wiring from per-module drum hooks to setHostHooks.
tests/drum_undo.test.mjs	Switches test wiring to setHostHooks (one stale comment remains).
tests/cross_arr_undo.test.mjs	Switches history wiring to setHostHooks.
tests/_history_env.mjs	Updates hook-tracking helper to install callbacks via setHostHooks.
src/main.js	Replaces multiple per-module hook wiring calls with a single setHostHooks({ ... }).
src/host.js	Adds shared host callback object and setHostHooks() setter, with rationale docs.
src/history.js	Replaces _hooks.* usage with host.* and removes setHistoryHooks.
src/drum.js	Replaces _hooks.* usage with host.* and removes setDrumHooks.
src/annotation-lanes.js	Replaces _hooks.* usage with host.* and removes setLaneHooks.
CHANGELOG.md	Adds an Unreleased entry describing the consolidation.

---

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

[byrongamatos]

All three fixed.

On the grep: worth noting the precise failure, because it is the same shape as the bug the comment is warning about. GNU grep accepts \s as an extension, so the command works on Linux — I checked. BSD/macOS grep does not; it matches a literal s, finds nothing, and reports a clean zero. A check whose entire job is to catch a reassignment returning a confident "no reassignments found" is the worst possible failure mode, so it is now a POSIX class with a note saying why.

Also swept the CHANGELOG: three other entries in the same Unreleased block still described the per-module setters this PR removes, so the release notes contradicted the code shipping with them.