feat: griff ingest — bulk-build corpus chunks from a folder of tabs

[PhysShell]

griff ingest — bulk-build corpus chunks from a folder of tabs

Turns a directory of scattered Guitar Pro / MIDI tabs into linked, rights-stamped phrase chunks in one pass — the candidate pile the corpus is curated from. Built toward the S12 precondition (a corpus of ≥ ~100 phrases), though those chunks are uncurated candidates: the curatorial tags (swancore style) and the reviewer decision are filled later in the cockpit. Chunks do carry the auto-derived technique tags build_chunk_meta reads from the notation (palm-mute, hammer-on, …) — facts from the tab, not curatorial choices.

Base	18ec861 (main)
HEAD	922dfcb
Commits	21 (10 RED → GREEN pairs + the GP7 main-merge)

Review addressed (CodeRabbit + Codex, 10 findings). Two P1s — chunks reloading as the wrong track, and sources never placed in the corpus — are fixed and round-trip-proven (HEAD 922dfcb): after ingest, griff generate --corpus loads the chunks (corpus: 38 chunks, 0 skipped) where before it skipped every one. See the review-response comment for the full map; schema is now v9 (SourceRef.track_index + sha256, additive).

What it does

griff ingest <dir> [--output corpus] [--with-bass] — for each file:

1. imports it (import_score_auto);
2. selects the guitar tracks (both, for the two-independent-guitar writing this corpus is full of), plus bass only with --with-bass, skipping drums/vocals;
3. phrase-splits every selected track;
4. links all its phrases to one per-file ensemble group (schema v4) so a reader can tell which chunks came from which tab;
5. stamps each with the default community-tab rights (copyrighted composition / community-tab-site / not redistributable);
6. writes the chunks + group.json, rebuilds the manifest, and prints a skip report (no guitar, import error, or no phrase survived splitting).

The heart, and the design that kept it small

The genuinely new logic is three pure, unit-tested griff_core::ingest functions:

function	tests
classify_track_role — role from name, channel 9, and open-string tuning; an all-C-1 placeholder tuning is not an instrument	11
select_ingest_tracks — guitars always, bass under the flag, order preserved	4
tuning_label — common tunings by name (standard_e, drop_d, …), the rest spelled c2_g2_c3_f3_a3_d4 so nothing is lost	6

The CLI side is composition, not new machinery: build_chunk_meta already accepts an ensemble link and a rights record, so phrase_chunks_for_track (a griff split that takes an explicit track instead of the first note-bearing one) sets the group link on each phrase after the split. No split-path signature changed and no existing test was touched — the first design threaded a new ensemble param through chunks_for_segments, which tripped too_many_arguments; setting the public ChunkMeta.ensemble field post-split is cleaner and inert to the existing griff split. assemble_ingest_group is the tested seam; cmd_ingest is thin I/O around it.

The classifier also protects the corpus: a track whose tuning can't be read (the C-1 placeholder) is classified Other and skipped, so a guitar chunk is never ingested with a bogus tuning. Those placeholder tunings are exactly the non-guitar tracks we drop.

RED → GREEN

RED	GREEN	pins
f2958d1	2ba4b4a	track-role classification
a4164e0	64c28dd	guitar/bass track selection
49cc577	5d2eae7	tuning label + spelling fallback
e6f78ae	dea1880	assemble one tab into linked, rights-stamped chunks; the griff ingest command wired on top
56950e5	d386446	acceptance-found: colliding filename stems get distinct group ids
66c8844	65e1d7b	acceptance-found: tuning label is order-agnostic (sort ascending)
c324b0e	f20cdd0	review P1: schema v9 track_index — a chunk reloads its exact source track, never substitutes
a866df5	859ea15	review P1 + fixes: ingest records track_index/sha256; part-index compaction; bass ≠ guitar naming
b87c10e	4a9d9d0	review: role by whole-word tokens, percussion before bass
7fb3bb0	3381e97	review P1: copy sources beside chunks (collision policy), loader verifies the hash; per-file skip; slugify fallback

The two acceptance-found pairs came from the first manual run, and the last four from the CodeRabbit + Codex review — both the sign of the process catching what reasoning didn't.

Why the fingering data matters, and is already safe

Preserving the real per-track tuning is not cosmetic: the model keeps each note's explicit Guitar Pro (string, fret) as NotePosition::explicit (gp.rs), and the tuning is the frame that makes those positions mean a pitch. That is exactly what a future fingering-aware generator needs, and it cannot be backfilled from pitches — so it is captured at ingest even though today's pitch-space S6/S7 generation does not read it. tuning_label is what records it legibly.

Manual acceptance (real run, files not committed)

griff ingest <tabs> --output <dir> over the real 410-file collection. GP7 .gp support is in this branch (via the merge of #133), so the .gp files ingest too:

ingested 401 files -> 9,909 phrase chunks in 401 ensemble groups (one per tab)
         401 sources copied beside their chunks
skipped   9:  6  no guitar or bass track
              3  truncated .gp5 (unexpected EOF)
formats:  gp5 5,561 · gp (GP7) 2,212 · gpx 1,401 · gp4 717 · gp3 18

9,909 phrases is far past the ~100-phrase S12 gate — the room is for curating down, not scraping for more. 359 of the 401 files yielded two or more guitar parts (the two-independent-guitar writing this targets).

A sample chunk (part 1 of a two-guitar tab) carries exactly what ingest promises — including the exact source track and content hash the loader needs (schema v9):

id          a_lot_like_birds_a_satire_of_a_satire_is_tiring_g1_p0
source      "A Lot Like Birds - A Satire Of A Satire Is Tiring.gpx"  (format gpx)
track_index 1                                   (the exact source track)
sha256      26b75c2c…                           (verified on load)
tuning      "standard_e"                        (named, order-agnostic)
ensemble    group_id "a_lot_like_birds_..."  part_index 1
rights      copyrighted_composition / community_tab_site / redistributable=false
tags        ["slide"]   reviewer null           (auto-derived technique tag; no curatorial tags/reviewer)

Round-trip verified — the acceptance that was missing the first time: after ingest, griff generate --corpus <dir> loads the chunks (corpus: 38 chunks, 0 skipped on a 3-tab corpus) where before the source-copy + schema-v9 fixes it skipped every one. Named tunings resolve order-agnostically: drop_d and standard_e dominate; the rest are extended-range tunings spelled consistently low-to-high.

Three defects surfaced only by the manual run — each fixed with its own RED → GREEN pair:

• colliding filename stems (a song present as both .gp5 and .gpx) overwrote chunks until unique_group_id;
• one physical tuning got two mirror-image labels depending on Guitar Pro's string order until tuning_label sorted ascending;
• (from review) multi-guitar chunks reloaded as the wrong track, and their sources were never placed in the corpus, until schema v9 (track_index + sha256) and the source-copy — see the review-response comment.

The corpus is copyrighted community tabs (redistributable: false), so no tab and none of these derived chunks are committed — corpus/ is git-ignored (ADR-0005). The end-to-end run is manual evidence, not a committed fixture test; the pure ingest logic and the strict loader are unit-tested on synthetic scores.

Validation

command	result
cargo fmt --all --check	0
cargo clippy --workspace --all-targets -- -D warnings	0
cargo test --workspace --exclude griff-cli	0 — core 231, cli-lib 21
cargo doc --no-deps --workspace	15 warnings, identical to base 18ec861
cargo +1.92 check --workspace --all-targets (MSRV)	0

cargo test -p griff-cli fails only missing_file_golden — the pre-existing Russian-locale golden (os error 2 renders localised on this machine), identical at base, green on CI's English runner.

Scope / not in this PR

• Uncurated by design. Chunks carry no curatorial tags and no reviewer decision (the auto-derived technique tags are facts from the tab, kept); the ≥100-phrase S12 gate wants curated phrases, so a human still tags/reviews them in the cockpit. This PR produces the candidate pile.
• Relations empty. The ensemble group records provenance (which chunks share a tab), not measured inter-guitar dependencies — the arbiter's A2 choice. Computing whole-track PairRelations is a trivial follow-up if wanted.
• No storage/DB layer. SQLite-vs-OPFS is a separate deferred decision, to be made once the derived corpus's shape and size are known — which this run now reveals.
• GP7 .gp support (PR feat(core): import Guitar Pro 7/8 (.gp) #133) is already in this branch via the main merge, so the 79 .gp files ingest here — no ordering dependency remains.

Not self-accepted, not merged.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NkqJUU6d1sW1RAfvyHrqVM

Summary by CodeRabbit

• New Features

  • Added an ingest command to bulk-process tab files into searchable chunk and ensemble records.
  • Automatically identifies guitar tracks and optionally includes bass tracks.
  • Generates stable tuning labels, ensemble links, collision-safe IDs, and refreshed corpus manifests.
  • Records skipped files with reasons and preserves source track details.

• Bug Fixes

  • Verifies source files against recorded content hashes to prevent loading mismatched files.
  • Uses the specified source track consistently during chunk preparation.

• Refactor

  • Updated corpus records to schema version 9 with enhanced source provenance metadata.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a bulk griff ingest command that classifies imported tracks, generates linked phrase chunks and ensemble groups, writes deterministic JSON records, and refreshes the manifest. Schema v9 stores source track and SHA-256 provenance used during loading.

Changes

Bulk ingest and provenance

Layer / File(s)	Summary
Track selection and tuning metadata core/src/lib.rs, core/src/ingest.rs	The public ingest module classifies tracks, selects eligible guitar and optional bass tracks, canonicalizes tuning labels, and tests these rules.
Source provenance schema and loading core/Cargo.toml, core/src/corpus.rs, core/src/generation_input.rs, core/tests/*, cli/src/generation_input.rs, ui-core/src/*, cockpit/web-test/*	Schema v9 adds optional source track and SHA-256 fields; loaders validate hashes and exact track selection, with updated fixtures and assertions.
CLI entrypoint and phrase linkage cli/src/main.rs	The CLI adds griff ingest, dispatches its arguments, and extends phrase generation to attach ensemble references and source provenance.
Group assembly and manifest refresh cli/src/main.rs	Ingest assembles per-file groups, creates collision-safe IDs, places source files, writes chunk and group JSON files, refreshes the manifest, and tests linkage and ID handling.

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

Sequence Diagram(s)

sequenceDiagram
  participant cmd_ingest
  participant select_ingest_tracks
  participant assemble_ingest_group
  participant phrase_chunks_for_track
  participant cmd_manifest
  cmd_ingest->>select_ingest_tracks: Select eligible guitar and optional bass tracks
  cmd_ingest->>assemble_ingest_group: Assemble chunks and ensemble group
  assemble_ingest_group->>phrase_chunks_for_track: Split tracks and attach ensemble references
  assemble_ingest_group-->>cmd_ingest: Return linked records
  cmd_ingest->>cmd_manifest: Refresh manifest after writing outputs

Loading

Possibly related PRs

• PhysShell/griff#69: Modifies the same build_chunk_meta path used for chunk metadata.
• PhysShell/griff#70: Modifies the phrase-chunk generation path refactored here.
• PhysShell/griff#102: Modifies the chunk-loading path extended with SHA-256 validation.

Suggested reviewers: claude

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the new griff ingest command that bulk-builds corpus chunks from a folder of tabs.

✨ 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/tab-ingest-track-roles

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 8

🤖 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 `@cli/src/main.rs`:
- Line 1552: Update the ingest flow surrounding used_ids so it is initialized
with IDs from existing groups before generating or collision-checking new IDs.
Ensure source-to-ID mapping is idempotent across repeated ingest runs, reusing
the existing group ID for an already-ingested source while assigning unique IDs
to new sources.
- Around line 1520-1535: Update slugify so stems containing no ASCII
alphanumeric characters return a stable, nonempty fallback ID instead of an
empty string, while preserving existing normalization for valid characters. Add
or update tests covering inputs such as "曲" and "---", including the resulting
group filename or chunk ID behavior.
- Around line 1483-1485: Update the track ID and title construction around the
visible id and title fields so optional bass tracks selected by --with-bass are
not labeled as guitars. Derive the suffix and display label from the track’s
TrackRole, while preserving the existing guitar naming for guitar parts and
using consistent bass or neutral naming for bass parts.
- Around line 1441-1450: Update the loop around phrase_chunks_for_track so
part_index values are assigned only to tracks that produce surviving chunks,
rather than using the selected iterator index. Track the next compact part index
and increment it after a track contributes chunks; preserve the shared index for
all chunks from that track. Add a regression test covering an empty first track
and verifying the next track receives part_index 0.
- Around line 1447-1449: Ensure the bulk-ingestion path around
phrase_chunks_for_track persists records with empty tags, despite
build_chunk_meta deriving technique, harmony, and syncopation tags. Add an
ingest-specific policy or clear those generated tags before records.push, while
preserving the existing metadata and member ID handling.
- Line 1587: Update the bulk ingestion loop around assemble_ingest_group to
handle per-file assembly errors like existing read/import failures: report the
failure for the current path, record it for the skip report, and continue
processing remaining files instead of propagating with ?. Preserve successful
record/group handling unchanged.

In `@core/src/ingest.rs`:
- Around line 49-64: Update the role classification logic around TrackRole to
tokenize normalized track names rather than using substring checks, ensuring
“bassoon” does not match bass. Evaluate explicit percussion and other
non-fretted tokens before bass so names such as “Bass Drum” return
TrackRole::Other, while preserving guitar and valid bass classification.
- Around line 71-90: The track-role classification around open_strings must not
use fallback Standard E tuning as guitar evidence. Update the ingest
classification flow to distinguish genuine/source-provided tuning from
Track.tuning’s default, and classify unnamed or unrecognized MIDI piano tracks
without tuning as Other (or preserve the appropriate source-aware role); retain
guitar classification only when genuine fretted evidence is available.

🪄 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: e1c40567-ed7e-48c6-9994-81bf153d4066

📥 Commits

Reviewing files that changed from the base of the PR and between 6cc24c9 and 65e1d7b.

📒 Files selected for processing (3)

• cli/src/main.rs
• core/src/ingest.rs
• core/src/lib.rs

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 65e1d7b233

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

[PhysShell]

Review addressed — HEAD 3381e97

All ten findings resolved (8 fixed in code as RED→GREEN pairs, 1 claim-corrected, 1 deferred with a reason). The two P1s were the load-bearing ones, and both are now verified end to end, not just by assertion.

The two P1s — fixed and round-trip-proven

The root cause both flagged: a chunk pointed into its source by filename + bar_range but not which track, and the sources were never placed in the corpus. My original acceptance verified chunks were written correctly but never loaded back through griff generate --corpus. That was the gap.

• P1 — wrong-part reload (c324b0e/f20cdd0, a866df5/859ea15): schema v9 adds SourceRef.track_index; prepare_chunk uses the exact named track and fails rather than substitute a silent or out-of-range one; pre-v9 records keep the legacy first-note-bearing fallback. Ingest records the exact source track on every chunk.
• P1 — sources not co-located (3381e97): cmd_ingest copies each source beside its chunks; load_chunk verifies the SHA-256 before importing.

Round-trip evidence (the acceptance that was missing): ingest three tabs → griff generate --corpus reports corpus: 38 chunks loaded, 0 skipped. Before the fix all 38 skip (no source on disk). At scale: 401 files → 9,909 chunks → 401 sources copied, each chunk carrying track_index + sha256.

The rest

#	finding	resolution
part_index gap	a selected track with no surviving phrase left a hole	compact the index; regression test for an empty first track (859ea15)
bass labelled guitar	_g/"(guitar)" applied to --with-bass parts	id tag + title derived from TrackRole (859ea15)
role by substring	"bassoon" → bass, "Bass Drum" → bass	whole-word tokens, non-fretted roles tested first (core, red+green)
per-file abort	one assembly error killed the run	caught and skipped like read/import errors (3381e97)
cross-run collisions	used_ids empty each run	seeded from the corpus's existing *.group.json (3381e97)
slugify ""	non-ASCII / punctuation stems	falls back to untitled + dedup (3381e97)
source hash	filename ≠ identity	sha256 pinned and verified on load (3381e97)

One claim corrected, not code

"Empty tags" was wrong — thank you. build_chunk_meta auto-derives technique tags (palm-mute, hammer-on, …) from the notation, so ingested chunks carry those. They are facts read from the tab, not curatorial choices, and I'd rather keep them than strip a free, correct signal. What the "uncurated candidate" claim actually means is no curatorial tags and no reviewer decision — the swancore style tags and the accept/reject a human picks are empty. I've corrected the PR wording accordingly.

One deferred, with a reason

MIDI without a tuning defaults to Standard E, so an unnamed MIDI piano track classifies as a guitar. Real: but out of this scope. Ingest targets Guitar Pro tabs, which carry genuine per-track tunings (the classifier already routes an unreadable/placeholder tuning to Other and skips it). A MIDI-safe classifier needs source-format-aware evidence (e.g. require explicit NotePosition), which is a separate refinement, not a fix to this bulk-GP path. Filed as a follow-up.

Validation

fmt · clippy -D warnings · test --workspace --exclude griff-cli (core 239, cli-lib 24) · cargo doc 15 = baseline · MSRV 1.92 · fuzz nightly — all clean. Schema v9 is additive: pre-v9 records round-trip byte-identically and load with the legacy fallback (pinned by a test).