Skip to content

feat(web): per-phrase fact-sheet in the split pager (#75) - #95

Merged
PhysShell merged 3 commits into
mainfrom
claude/modest-maxwell-0zecwn
Jun 20, 2026
Merged

feat(web): per-phrase fact-sheet in the split pager (#75)#95
PhysShell merged 3 commits into
mainfrom
claude/modest-maxwell-0zecwn

Conversation

@PhysShell

@PhysShell PhysShell commented Jun 20, 2026

Copy link
Copy Markdown
Owner

Re-aims the open part of #75 away from song-form labels (verse/chorus) toward a transferable phrase fingerprint: the split pager now shows each phrase's intrinsic characteristics under the info line.

Why (design note)

Section labels are bound to the source song and lose meaning once a phrase enters the corpus — a "chorus" in one song and a "verse" in another can be musically identical. Intrinsic characteristics — register, density, dynamics — are what generation/similarity actually want, and they're computable from data the split already emits. (Discussed with the maintainer; this reframes #75's "structure tags" sub-ask rather than deriving verse/chorus.)

What

A compact per-phrase fact-sheet, e.g.:

E2–A4 · 29 st · 6.5 notes/bar · vel 48–124
D3–D3 · 0 st · 2 notes/bar · vel 100–101 (flat)
  • Register — low–high notes (scientific pitch) + semitone span
  • Density — notes per bar
  • Dynamics — velocity range + a (flat) hint (a hand for the existing flat_dynamics quality flag)

How

  • New pure helper web/static/phrasestats.jsnoteName / phraseStats / formatStats, computed from the split's {p,s,d,v} notes. No engine/wasm change: it reads JSON the split already produces (notes, bar_lo, bar_hi).
  • Wired into renderPhrase() as a new #splitStats line under the phrase info; tinted part-A blue, monospaced to read as a data strip.

TDD

red (1d9fca9) → green (1d97839) → glue (8d0fd3e). The helper is unit-tested; the single render line is DOM glue (exempt, ADR-0024).

Verification

  • 36/36 web JS tests green (node --test from web/), incl. 7 new for noteName / phraseStats / formatStats
  • node --check web/static/app.js clean
  • Static front-end only — no Rust/wasm touched

⚠️ Needs an in-browser eyeball. The render line isn't unit-tested (ADR-0024) — confirm the strip's placement/look via ./web/build.sh + serving dist/ (load sample → Split). Logic is fully covered.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95


Generated by Claude Code

Summary by CodeRabbit

  • New Features

    • Added a phrase statistics panel to the split-view editor displaying key metrics for each phrase, including note count, pitch range, velocity range, dynamic characteristics, and per-bar note density.
  • Tests

    • Added comprehensive unit tests verifying phrase statistics computation and display formatting functions.

claude added 3 commits June 20, 2026 14:42
#75)

Failing unit tests for a pure phrase fact-sheet: scientific note names,
register low/high/span, notes-per-bar density, and a velocity-range +
flat-dynamics summary — all computed from the split's `{p,s,d,v}` notes,
so the web pager can show a phrase's musical fingerprint instead of a
verse/chorus label. The module is a stub here; the green commit implements it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
`noteName` (scientific pitch), `phraseStats` (register low/high/span,
notes-per-bar density, velocity range + flat-dynamics flag), and
`formatStats` (compact one-liner). Pure functions over the split's
`{p,s,d,v}` notes — no DOM, no wasm. Also corrects a test expectation:
MIDI 57 is A3, not A4.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
Wires the phrasestats helper into renderPhrase: under the phrase info line
the pager now shows the shown phrase's register (low–high + semitone span),
notes-per-bar density, and velocity range + a flat-dynamics hint — an
intrinsic, transferable fingerprint of the phrase rather than a song-form
label. Static front-end only (app.js/index.html/style.css); the new line is
tinted part-A blue and monospaced to read as a compact data strip.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Review Change Stack

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: beab3577-9ca0-45b6-a2af-2238c676db00

📥 Commits

Reviewing files that changed from the base of the PR and between b18a308 and 8d0fd3e.

📒 Files selected for processing (5)
  • web/static/app.js
  • web/static/index.html
  • web/static/phrasestats.js
  • web/static/style.css
  • web/test/phrasestats.test.js

📝 Walkthrough

Walkthrough

A new DOM-free helper module web/static/phrasestats.js is added with three exports: noteName, phraseStats, and formatStats. The split-view HTML gains an output#splitStats element, style.css styles it, and app.js imports the helpers and calls them inside renderPhrase() to populate the element. A unit test file covers all helpers and edge cases.

Changes

Per-phrase stats panel

Layer / File(s) Summary
phrasestats.js helper module and tests
web/static/phrasestats.js, web/test/phrasestats.test.js
New module exports noteName(midi) (MIDI integer → scientific pitch name), phraseStats(notes, bars) (computes count, register range, span, per-bar density, velocity range, flat-dynamics flag; returns null for empty/zero-bar input), and formatStats(st) (serializes to a compact string or ''). Unit tests cover all helpers, including sharp notes, flat-dynamics threshold, zero-bar divide-by-zero guard, and null input for formatStats.
HTML element, CSS styling, and app.js wiring
web/static/index.html, web/static/style.css, web/static/app.js
output#splitStats added to split-view markup. #splitStats styled with block layout, centered monospace text, small font size, and accent color. app.js imports the two helpers, registers els.splitStats, and renders the formatted fact-sheet inside renderPhrase() when the element exists.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • PhysShell/griff#71: Also modifies renderPhrase() in app.js to extend per-phrase UI state in the split-pager, adding chunk navigation, play, and download controls.
  • PhysShell/griff#82: Also extends renderPhrase() in the split-phrase UI, adding compare-track overlay and summary via a compareTracks helper.
  • PhysShell/griff#85: Also modifies renderPhrase() to conditionally append duplicate-phrase info to the split-view output element.

Poem

🐇 A hop, a skip, a MIDI note,
I count the bars and check the flat,
E2 to G4, my fact-sheet wrote,
Velocity range? I know all that!
With monospace font and accent blue,
Each phrase reveals its stats to you~ ✨

🚥 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 accurately and concisely describes the main change: adding a per-phrase fact-sheet feature to the split pager UI, which is the primary focus across all five modified/added files.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% 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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 claude/modest-maxwell-0zecwn

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants