feat(editor): one consolidated ruler + whole-song minimap (workspace-shell B3)

[ChrisBeWithYou]

What

Workspace-shell B3 (charrette §2.5 / D-C5): the three time-surfaces — the floating HTML loop strip, the waveform band's seek role, and the bottom beat bar — consolidate into a canvas timeline header in the charrette's locked order (transport+LCD → ruler → waveform → lanes → sub-lanes):

• Minimap (14px, whole-song space): sections as tint blocks, the loop region, the viewport window, the playhead tick. Click/drag pans the view anywhere in the song — the overview the editor lacked (concept adopted from Virtuoso's overview canvas per the charrette's port table; own implementation on the audio-anchored timeline).
• Ruler (26px, chart space): the one authoritative strip owning bars + beats + sections + loop + playhead. Upper half = the loop lane (drag paints a region, mode-aware — Bar/Grid/Free per the loop-snap pref, Shift = free, live mid-drag; edge grips resize through the same _loopEdgeAdjustPure the strip used). Lower half = scrub (press/drag the playhead; playback resumes after). Measure numbers skip in powers of two instead of colliding at zoom-out; sub-beat ticks appear once a beat clears 6px. Section names moved here from the lane area (the dashed boundary lines stay in the chart as guides).

Everything the ruler paints is a pure function of S.beats through timeToX — the D-T1 invariant as chrome.

What moved where

• Loop state is untouched: still S.barSel, same beat-anchored edges (A4), same commands, same undo, same Loop-in-3D handoff. Only the surface moved. _renderLoopStrip survives as the "loop surface changed → refresh mirrors + repaint" hook (tempo/grid ops call it), minus the DOM.
• Loop snap mode (Bar/Grid/Free) + Clear loop → Transport ▸ Loop menu rows (new loopSnap/loopClear item kinds resolve checkmarks at open time, like everything else in the B4 menu).
• The strip handles' arrow-key nudge → Alt+←/→ nudges the loop start, Alt+Shift+←/→ the end (same _loopNudgeEdge, same mode resolution; direct input.js handling per the drum-velocity-nudge precedent).
• Beat-bar drag-select → the ruler's loop half (and it's mode-aware now, where the beat bar was bar-only).
• The waveform band keeps click-to-seek as a convenience; the ruler is the authoritative scrub surface.

The layout shift

Every band below the header moves down by a fixed TIMELINE_TOP (40px): laneToY/yToLane/midiToY carry it, the retired beat bar's BEAT_H is deleted (lanes reclaim its height), _beatBarTopY() keeps its name as the lanes-bottom anchor every sub-lane and hit-test already uses. Hit-testing migrated: the header consumes every press above TIMELINE_TOP in every mode (the old strip was mode-independent DOM — drum/tempo modes keep their loop editing); drum/tempo/parts/keys-gutter/dblclick gates all shifted and suite-verified.

Tests

tests/ruler.test.mjs (13, real-import ESM; fails on main — src/ruler.js doesn't exist): zone split, minimap map round-trip + clamps + degenerate durations, label-skip, edge-grab resolution, and interaction round-trips against the real S (loop create carries the mode, edge drag resolves to whole bars, scrub tracks + clamps, minimap pans, header consumption, Shift=free). Geometry/waveform/gutter suites updated where they pinned the old y-space (sliced suites inject TIMELINE_TOP=0 so their fixtures stay exact); menu_model passes unchanged.

Full suite 102/102; ESLint clean (0 errors, warn-ratchet unchanged).

Expected keep-both seams vs the other open PRs: CHANGELOG (all of #205–#208), src/menu-bar.js dispatch/menu rows (#205), screen.html (#205).

🤖 Generated with Claude Code

https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q

Summary by CodeRabbit

• New Features

  • Added a consolidated timeline header with whole-song minimap and ruler (loop painting/resizing and playhead scrubbing live on the ruler).
  • Transport ▸ Loop now includes Clear loop and Loop snap (Bar/Grid/Free).
  • Alt+ArrowLeft/ArrowRight nudges loop start/end (Shift for variants; Ctrl for coarse steps).

• Improvements

  • Re-aligned lanes, annotations, tempo/visual guides, section/measure labeling, and cursor behavior to the new header layout; fixed a header-related layout shift while keeping waveform click-to-seek.

• Tests

  • Added ruler/minimap/loop interaction tests and updated geometry/rendering expectations.

[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: 5c00c745-c6ac-4b2b-9942-c6d4e440691f

📥 Commits

Reviewing files that changed from the base of the PR and between 493eaa3 and 378c6fb.

📒 Files selected for processing (5)

• CHANGELOG.md
• src/drum.js
• src/input.js
• src/ruler.js
• src/tempo.js

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

• CHANGELOG.md

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

• src/drum.js
• src/input.js
• src/tempo.js

---

📝 Walkthrough

Walkthrough

The editor replaces the standalone loop strip and beat bar with a consolidated minimap/ruler header. Loop editing, scrubbing, snapping, keyboard nudging, menu actions, canvas geometry, rendering, and related tests are updated for the new 40px timeline offset.

Changes

Timeline editor

Layer / File(s)	Summary
Header geometry, rendering, and interactions src/geometry.js, src/ruler.js, tests/ruler.test.mjs	Adds minimap/ruler geometry, drawing, scrubbing, panning, loop editing, and interaction coverage.
Loop controls and event routing screen.html, src/loop.js, src/input.js, src/main.js, src/menu-bar.js, src/mouse.js	Moves loop interactions from the DOM strip to the ruler and adds menu-based clear/snap actions plus keyboard edge nudging.
Canvas coordinate migration src/annotation-lanes.js, src/draw.js, src/drum.js, src/keys.js, src/parts-view.js, src/tempo.js, src/waveform.js	Rebases drawing and hit-testing around TIMELINE_TOP, removes the beat-bar reserve, and updates lane extents.
Host integration and validation src/host.js, tests/geometry.test.mjs, tests/keyboard_gutter_dblclick.test.mjs, tests/waveform_render.test.js, CHANGELOG.md	Adds timeline-header host wiring and updates regression fixtures and changelog documentation.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant Canvas
  participant Ruler
  participant EditorState
  participant Renderer
  User->>Canvas: drag minimap, ruler, or scrub area
  Canvas->>Ruler: route header pointer event
  Ruler->>EditorState: update scroll, cursor, or loop selection
  Ruler->>Renderer: request redraw

Loading

Possibly related PRs

• got-feedBack/feedBack-plugin-editor#92: Overlaps loop snap-mode and loop-edge behavior.
• got-feedBack/feedBack-plugin-editor#149: Overlaps shared geometry metrics and lane-to-Y mapping.
• got-feedBack/feedBack-plugin-editor#186: Overlaps canvas mouse routing and extracted pointer handlers.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 39.39% 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 summarizes the main change: a consolidated ruler and whole-song minimap for workspace-shell B3.
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 feat/editor-ruler-consolidate

---

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

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/parts-view.js (1)

149-204: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Render the shared timeline header in Parts View src/parts-view.js:149-204 — _partsViewDraw() only paints host.drawWaveform(w) plus the part lanes, so the 0..TIMELINE_TOP strip stays empty. If Parts View should match the main editor, draw the minimap/ruler here too.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/parts-view.js` around lines 149 - 204, Update _partsViewDraw() to render
the shared timeline header across the 0..TIMELINE_TOP region before drawing the
part lanes, matching the main editor’s minimap/ruler rendering. Reuse the
existing shared header-drawing routine and preserve the current waveform and
lane rendering behavior.

🧹 Nitpick comments (2)

tests/ruler.test.mjs (2)

144-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Missing coverage for scrub-while-playing / resume.

seedGrid() sets playing: false for every test, so no test exercises the resume path in rulerOnMouseDown/rulerOnMouseUp (starting a scrub while S.playing is true). Given the resume flag's correctness is in question (see src/ruler.js comment), a test seeding S.playing = true before a scrub press/release would catch this.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/ruler.test.mjs` around lines 144 - 153, Extend the scrub test around
rulerOnMouseDown and rulerOnMouseUp to set S.playing = true before the press,
then verify scrubbing records the resume state and restores playback after
release. Keep the existing seek, drag-type, movement, and cleanup assertions
intact.

---

117-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Dead/tautological assertion — test doesn't verify what its comment claims.

S.barSel = null; const region = { startTime: 4.5, endTime: 9.2 }; ... assert.strictEqual(rulerOnMouseUp(), false, 'barsel up belongs to mouse.js, not the ruler'); S.drag = null; assert.ok(region.endTime > region.startTime); — region is a locally-constructed literal never derived from ruler behavior; asserting region.endTime > region.startTime is always true regardless of what the ruler does. The "Drag right into bar 3 — the region snaps to whole bars 2–3" comment implies snapping is verified here, but it isn't — this only tests the mousedown seed and that rulerOnMouseUp returns false for barsel.

Either drop the dead region local/comment, or actually exercise _loopRegionForDragPure (the same pure the mouse.js move path calls) to verify the claimed snap-to-bar behavior.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/ruler.test.mjs` around lines 117 - 129, Remove the tautological local
region construction and assertion from the loop-half drag test, or replace them
by invoking _loopRegionForDragPure with the drag coordinates to assert the
expected whole-bar 2–3 region. Keep the existing mousedown barsel setup and
rulerOnMouseUp ownership assertion, and ensure the test directly verifies the
snapping behavior described by its comment.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/input.js`:
- Around line 905-912: Update the tone-lane context-menu gate in onContextMenu
to accept y values from TIMELINE_TOP through TIMELINE_TOP + TONE_LANE_H,
replacing the zero-based range while preserving the existing tone-menu handling.

In `@src/ruler.js`:
- Around line 308-313: In the scrub handling flow, capture the current S.playing
value before calling stopPlayback(), then assign that saved value to
S.drag.resume. Preserve the existing scrubTo(x) behavior and ensure mouse-up can
resume playback when it was active before scrubbing.

---

Outside diff comments:
In `@src/parts-view.js`:
- Around line 149-204: Update _partsViewDraw() to render the shared timeline
header across the 0..TIMELINE_TOP region before drawing the part lanes, matching
the main editor’s minimap/ruler rendering. Reuse the existing shared
header-drawing routine and preserve the current waveform and lane rendering
behavior.

---

Nitpick comments:
In `@tests/ruler.test.mjs`:
- Around line 144-153: Extend the scrub test around rulerOnMouseDown and
rulerOnMouseUp to set S.playing = true before the press, then verify scrubbing
records the resume state and restores playback after release. Keep the existing
seek, drag-type, movement, and cleanup assertions intact.
- Around line 117-129: Remove the tautological local region construction and
assertion from the loop-half drag test, or replace them by invoking
_loopRegionForDragPure with the drag coordinates to assert the expected
whole-bar 2–3 region. Keep the existing mousedown barsel setup and
rulerOnMouseUp ownership assertion, and ensure the test directly verifies the
snapping behavior described by its comment.

🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7362887b-2d78-4ff5-b29b-61f5c8a21675

📥 Commits

Reviewing files that changed from the base of the PR and between 1242631 and f52c295.

📒 Files selected for processing (20)

• CHANGELOG.md
• screen.html
• src/annotation-lanes.js
• src/draw.js
• src/drum.js
• src/geometry.js
• src/input.js
• src/keys.js
• src/loop.js
• src/main.js
• src/menu-bar.js
• src/mouse.js
• src/parts-view.js
• src/ruler.js
• src/tempo.js
• src/waveform.js
• tests/geometry.test.mjs
• tests/keyboard_gutter_dblclick.test.mjs
• tests/ruler.test.mjs
• tests/waveform_render.test.js