feat(web): show detected phrase tags + export all phrases as one JSON by PhysShell · Pull Request #79 · PhysShell/griff

PhysShell · 2026-06-19T07:35:03Z

Two ergonomics fixes from a playground playtest (Dance Gavin Dance GPX). Partially addresses #74 and #77; the rest of the playtest backlog is #73–#78.

What

Detected tags are now visible per phrase. The split pager rendered only a tag count, so notation-derived techniques (slide, hammer_on…) looked like they "weren't recognized." Each phrase now shows its resolved tags as ticked, read-only chips (tinted part-A blue to set them apart from the green capture inputs). Read-only on purpose — they reflect the phrase and do not feed the next split. (web: surface detected tags, tuning, and instrument in the curation UI #74)
Export all phrases as one JSON. The ⬇ all phrases button saved N separate files (staggered); it now downloads a single <id>.chunks.json — a JSON array of every phrase chunk. The base id defaults from the capture form (strips the _p<N> suffix). (web: export all phrases as one JSON; avoid chunk-id collisions across sessions #77)

Scope

Static front-end only (web/static/app.js, index.html, style.css) — no engine/Rust change.

Verification

Headless e2e (Playwright + Chromium) against the real wasm build and the real GPX: loads the tab → 13 phrases (bar ranges match the engine), tag chips render for 10/13 phrases (all ticked + disabled), 3 show "no tags"; ⬇ all (one JSON) produces a single 13-element array. No page errors.
node --check on app.js; the debug-log unit lane stays 8/8 (web-test).

Note

The e2e also surfaced an unrelated UX bug now logged on #73: loading this GPX with the default mode (counter_melody) shows error: Arrange(NonUniformTimeline) because the song changes meter. Not addressed here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95

Generated by Claude Code

Summary by CodeRabbit

New Features
- Detected phrase tags are now shown in the phrase panel as read-only checkbox indicators, including a muted “no tags detected” state.
Improvements
- “Download all” now produces a single .chunks.json file containing an array of all phrase chunks.
- “Download each phrase” has a dedicated download flow to keep individual file saving stable.
- Updated the “download all” button label and helper text to reflect the one-JSON format.

Two playtest fixes (parts of #74, #77): - Per-phrase tags are now visible: the split pager renders each phrase's resolved tags (chosen + notation-derived techniques like slide/hammer_on) as ticked, read-only chips instead of just a count. Read-only on purpose — they reflect the phrase and do not feed the next split. - "all phrases" now downloads a single JSON array (<id>.chunks.json) instead of N separate files; the base id defaults from the capture form (strips _p<N>). Static front-end only; the debuglog test lane stays 8/8.

coderabbitai · 2026-06-19T07:35:16Z

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: cb64b7fd-300b-4453-8334-cb32f838873b

📥 Commits

Reviewing files that changed from the base of the PR and between 1fabea9 and b0cfbaa.

📒 Files selected for processing (2)

web/static/app.js
web/static/index.html

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

web/static/index.html
web/static/app.js

📝 Walkthrough

Walkthrough

The split pager UI gains a read-only div#splitTags panel that renders each phrase's resolved tags as checked/disabled checkboxes (or an empty-state message). The "download all" action is refactored into two flows: staggered per-phrase .chunk.json downloads and a single .chunks.json file containing a JSON array of all phrase chunks. Button labels and hint text are updated accordingly.

Changes

Split pager: per-phrase tag display and bundled download

Layer / File(s)	Summary
Tag panel HTML, CSS, and DOM binding `web/static/index.html`, `web/static/style.css`, `web/static/app.js`	`div#splitTags` container added to `#splitView` for displaying phrase tags, `.tagsread` CSS rules define spacing and checkbox accent color, and `splitTags` registered in the `els` DOM bindings map. Download button label updated to "⬇ all (one JSON)".
`renderPhraseTags` implementation and phrase pager update `web/static/app.js`	`renderPhrase()` now calls `renderPhraseTags(tags)` to populate `els.splitTags` with read-only checked checkboxes or a muted empty-state message.
Separate download handlers `web/static/app.js`	`downloadEachPhrase()` saves individual `${id}.chunk.json` files with staggered timing; `downloadBundle()` collects all phrase chunks into a single `${base}.chunks.json` array with parsed chunks or `{id, error}` fallback for unparseable entries.
Event wiring and documentation `web/static/app.js`, `web/static/index.html`	Button handlers wired so `splitDownloadEach` calls the per-phrase downloader and `splitDownloadAll` calls the bundled downloader. Hint text updated to reflect "each phrase" vs "all (one JSON)" output shapes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

web: export all phrases as one JSON; avoid chunk-id collisions across sessions #77 — The PR implements the feature to export all phrases as a single JSON array file (${base}.chunks.json) instead of multiple individual files.

Possibly related PRs

PhysShell/griff#71 — Introduces the split pager phrase rendering and download handlers that this PR refactors and extends.
PhysShell/griff#72 — Modifies the same phrase chunk parsing and tag surfacing code paths in the split pager UI.

Poem

🐇 One JSON file for all my phrase-y dreams,
No more staggered downloads flooding downstream.
Tag checkboxes glow, read-only and neat,
A bundled array makes the data complete.
The split pager hops—unified, oh how sweet! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.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 accurately summarizes the two main changes: displaying detected phrase tags in the UI and adding a new export option for all phrases as a single JSON file, which align with the pull request objectives.
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.}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1fabea9b87

ℹ️ 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".

chatgpt-codex-connector · 2026-06-19T07:37:28Z

+  const chunks = splitChunks.map((ch) => {
+    try { return JSON.parse(ch.chunk); } catch (_) { return { id: ch.id, error: 'unparseable chunk' }; }
+  });
+  saveBlob(`${base}.chunks.json`, JSON.stringify(chunks, null, 2));


Keep the all-phrase export manifest-readable

When a curator clicks ⬇ all (one JSON) after splitting phrases and drops the downloaded file into the corpus folder, this now writes a .chunks.json file containing an array, but the documented capture flow still sends these downloads to griff manifest, and cmd_manifest only collects filenames ending in .chunk.json and deserializes each file as a single ChunkMeta (cli/src/main.rs lines 1109-1132). The old all-phrases button produced files that the manifest consumed; this replacement silently leaves every phrase out of the manifest unless another importer is added or the export remains a set/archive of individual .chunk.json records.

Useful? React with 👍 / 👎.

Good catch — fixed in b0cfbaa. You're right that cmd_manifest collects individual *.chunk.json files (one ChunkMeta each), so the array bundle would be silently skipped.

Rather than regress that path, I kept both, clearly labeled:

⬇ each phrase → one <id>_p<N>.chunk.json per phrase (manifest-ready — the previous behavior, restored)

⬇ all (one JSON) → a single <id>.chunks.json array (for sharing/review)

The bundle's .chunks.json suffix doesn't match manifest's .chunk.json filter, so it's safely ignored rather than mis-parsed. Teaching griff manifest to also ingest a .chunks.json array is captured as a follow-up on #77. Verified with a headless e2e (13 phrases, tag chips, bundle = 13-element array, no page errors).

Generated by Claude Code

Codex P2 (#79): replacing the per-file "all phrases" export with a single .chunks.json array broke the documented capture -> `griff manifest` flow — cmd_manifest (cli/src/main.rs) ingests individual *.chunk.json files, one ChunkMeta each, and would silently skip the array bundle. Keep both, clearly labeled: - "each phrase" -> one <id>_p<N>.chunk.json per phrase (manifest-ready) - "all (one JSON)" -> a single <id>.chunks.json array (sharing/review) Verified: node --check, debuglog lane 8/8, headless e2e (13 phrases, tags render, bundle is a 13-element array, no page errors).

chatgpt-codex-connector Bot reviewed Jun 19, 2026

View reviewed changes

PhysShell mentioned this pull request Jun 19, 2026

web: export all phrases as one JSON; avoid chunk-id collisions across sessions #77

Closed

PhysShell merged commit 96df613 into main Jun 19, 2026
2 checks passed

PhysShell mentioned this pull request Jun 19, 2026

feat(web): default chunk id from the file + warn on stale-id collision (#77) #86

Merged

Navigation Menu

feat(web): show detected phrase tags + export all phrases as one JSON - #79

feat(web): show detected phrase tags + export all phrases as one JSON#79
PhysShell merged 2 commits into
mainfrom
claude/modest-maxwell-0zecwn

PhysShell commented Jun 19, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 19, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jun 19, 2026

Uh oh!

PhysShell Jun 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

PhysShell commented Jun 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Scope

Verification

Note

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

PhysShell Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

PhysShell commented Jun 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 19, 2026 •

edited

Loading