feat: griff split in the browser — paginated per-phrase chunks (auto-split #2b: web) - #71
Conversation
Auto-split #2b: the browser capture tool should turn one track into one corpus chunk per detected phrase (mirroring CLI 'griff split'), so the page can paginate and play each phrase. Add failing tests for the split seam — renumbered sounding phrases with inclusive bar_range, dropping phrases silent on the detected track (single-track contract), out-of-range errors, and an end-to-end split of the built-in sample. Private stubs keep the existing suite running.
Adds split_chunks_json — the browser twin of CLI 'griff split'. Reusing core's now-public extract_bars + bar_segments, it cuts the selected track at its detected phrase boundaries, slices each segment, and measures it on the same detected track via the existing build_chunk_meta_record. Segments silent on that track are dropped (single-track contract); survivors renumber from 0 with inclusive bar_range and inherit the capture form's metadata (ids/titles suffixed _p<N>). Each entry carries its notes for playback and a download-ready pretty chunk.json. Verified on host tests and the wasm32 build.
Wires split_chunks_json into the capture panel: a Split-into-phrases button runs the split over the selected track with the capture form's metadata, then a pager (Prev / phrase n/N / Next) walks the resulting chunks. Each phrase reuses the shared piano roll and transport synth for per-phrase playback, and downloads as its own <id>.chunk.json (one, or all). A new file or track selection resets the split. Generation (arrange) is untouched.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a browser-side "auto-split" capture flow: a new ChangesWeb auto-split phrase chunking
Sequence Diagram(s)sequenceDiagram
participant User
participant AppJS as app.js
participant WASM as split_chunks_json (WASM)
participant Core as griff_core
User->>AppJS: click "Split into phrases"
AppJS->>WASM: split_chunks_json(track, id, title, ...)
WASM->>Core: bar_segments(boundary_ticks, bars)
Core-->>WASM: Vec<Range<usize>>
loop each segment
WASM->>Core: extract_bars(score, range)
Core-->>WASM: sliced Score
WASM->>WASM: note_count filter (drop silent phrases)
WASM->>WASM: build ChunkMeta, set inclusive bar_range
end
WASM-->>AppJS: {"error":null,"chunks":[{notes, chunk, id, title}, ...]}
AppJS->>AppJS: store splitChunks[], renderPhrase(0)
AppJS-->>User: show splitView pager (phrase 1/N)
User->>AppJS: click Prev / Next
AppJS->>AppJS: stepPhrase(±1), renderPhrase(idx)
User->>AppJS: click Download / Download All
AppJS->>AppJS: saveBlob(chunk JSON, filename)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@web/src/lib.rs`:
- Around line 688-689: The functions note_count() and push_notes() currently
only examine the first voice in a track (voices.first()), causing tracks with
notes in secondary voices to be incorrectly treated as silent and dropped during
phrase splitting. Create split-specific helper functions that iterate through
all voices using track.voices.iter() to check for notes across all voices, not
just the primary voice. Apply these new helpers where note_count() is used in
the track matching logic around line 688, and add a regression test that
verifies a track with an empty voice 0 and note-bearing voice 1 is correctly
retained and processed.
🪄 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: 14698387-cc56-4e8e-a3f6-c07be419e5e8
📒 Files selected for processing (6)
docs/decisions.log.mdweb/README.mdweb/src/lib.rsweb/static/app.jsweb/static/index.htmlweb/static/style.css
Characterization test for the existing behavior CodeRabbit flagged on #71: the split's note_count/push_notes read voice 0 because the whole analysis stack — boundary::detect_phrase_boundaries, structure/gesture/complexity, novelty/closure/complement, MIDI export — reads voices.first(). Phrases are cut from voice 0, so a track sounding only in a secondary voice is unmeasurable and is dropped, matching the CLI's primary_voice_note_count contract. Scanning all voices would cut on voice 0 yet keep/measure on voice 1 and reintroduce the measurement-less chunks PR #70's Codex P2 removed.
|
@codex review (CodeRabbit is clean — its one comment was withdrawn as the intended voice-0 convention. Requesting a Codex pass before merge, as on the #2a PR #70.) Generated by Claude Code |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9edf10dfa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| /* ---- auto-split phrase pager (#2b) ---- */ | ||
| .splitview { | ||
| display: grid; |
There was a problem hiding this comment.
When the page first loads, or when resetSplit() sets splitView.hidden = true after a track/file change, this author display rule can override the browser's default [hidden] styling and still render the empty pager/download controls. Make the grid display conditional on the element not being hidden (for example .splitview:not([hidden])) or add an explicit [hidden] { display: none !important; } rule so the split UI only appears after a successful split.
Useful? React with 👍 / 👎.
2 participants
Second half of auto-split (feature #2), the web side, following #70 (#2a:
core + CLI). The browser capture tool could only emit one chunk per whole
track; this turns a loaded track into one corpus chunk per phrase — the
browser twin of
griff split— and lets you page through, play, and downloadeach phrase.
What
WASM (
web/src/lib.rs)split_chunks_json(track, …capture-meta) -> String— mirrors the CLI'sgriff split. Reuses core's now-publicslice::extract_bars+split::bar_segmentsand the existing webbuild_chunk_meta_record: it cutsthe selected track at its detected S4 phrase boundaries, slices each segment,
and measures it on the same detected track. A segment silent on that track
is a phrase rest and is dropped — never re-measured on another track
(single-track contract; this is the Codex P2 fix from feat: griff split — one corpus chunk per phrase (auto-split #2a: core + CLI) #70, carried into the
web split and covered by a web test). Survivors renumber from 0 with inclusive
bar_range[start, end-1]and inherit the capture form's metadata(ids/titles suffixed
_p<N>/(phrase <N>)).{error, chunks:[{id, title, bar_lo, bar_hi, notes, chunk}]}, whereeach
chunkis a download-readychunk.json(a serializedcorpus::ChunkMeta,byte-shape identical to what
griff manifestreads) andnotesdrives playback.Frontend (
web/static/)the capture form's metadata, then a pager (◀ Prev · phrase n/N · Next ▶)
walks the chunks. Each phrase reuses the shared piano roll and transport synth
for per-phrase playback, and downloads as its own
<id>.chunk.json(one,or all). A new file or track selection resets the split.
arrange) is untouched.Design note
The per-phrase primitives (
extract_bars,bar_segments) are shared via core;the per-phrase assembly is mirrored in CLI and web exactly as
build_chunk_meta_recordalready mirrors the CLI'sbuild_chunk_meta— the twofronts read different curate inputs (CLI prompts vs JS string args). Both fronts
now test the track-consistency rule so they stay in step. (decisions.log,
2026-06-18.)
Tests (strict TDD)
e040bc1red →8511fe4green for the split seam(
split_segments_to_json/split_to_json): renumbered sounding phrases withinclusive
bar_range, dropping phrases silent on the detected track,out-of-range errors, and an end-to-end split of the built-in sample. Private
stubs keep the existing suite running in the red commit.
Validation
cargo test(web, host) green;cargo clippy --all-targets -- -D warningsclean.
./web/build.sh(966 KiB);split_chunks_jsonexported in the generated glue.
node --checkonapp.jsand HTML/JS idparity verified.
web/diststays gitignored (CI rebuilds on deploy).fully unit-tested and the JS is thin glue verified structurally.
🤖 Generated with Claude Code
https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
Generated by Claude Code
Summary by CodeRabbit
chunk.jsonper detected phrase for a loaded trackbar_rangeand correct chunk serialization/deserialization