feat: auto-derive technique tags from notation in curation - #69
Conversation
Guitar Pro import already records techniques explicitly (ADR-0018), so curation should read them instead of re-asking a human. Pins derive_techniques(score, track): spans/marks with a dedicated SwancoreTag become tags (hammer-on, palm-mute, pinch→artificial harmonic, …); the free-form `techniques` list is the superset (legato, accent, … too). References griff_core::technique, which does not exist yet — the suite fails to compile until the green step. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
griff_core::technique::derive_techniques(score, track) reads the track's primary voice (voice 0 — the convention structure/gesture/boundaries follow) and returns the techniques its notation states are present: - tags: SwancoreTags with a dedicated per-occurrence variant (hammer-on, pull-off, slide, bend, vibrato, palm-mute, natural/artificial harmonic); - names: the superset of lower_snake_case names, also recording legato, accent, ghost, staccato, dead-note and tap, which have no dedicated tag. Presence-only (no thresholds), so it stays a pure function of the score (SPEC §6). The passage-level TappingPassage/LegatoPassage tags are about dominance, not presence, and are intentionally left out. 4/4 technique_tags tests pass; clippy -D warnings (incl. nursery) clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
Keeps the curator's order, then appends any derived tag not already present. Stable and idempotent, so auto-derivation adds technique tags without overriding choices. The shared seam both `griff curate` and the web capture tool use to fold derive_techniques output into ChunkMeta.tags. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
build_chunk_meta now derives techniques from the selected track's notation (technique::derive_techniques) and folds them in: ChunkMeta.techniques is the free-form name list (was always empty), and tags merge the curator's choices with the derived technique tags (technique::merge_tags). The curator no longer re-tags by hand what the Guitar Pro file already states. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
build_chunk_meta_record now folds technique::derive_techniques into the chunk: `techniques` carries the free-form name list (was always empty) and the tags merge the curator's checkbox picks with the derived technique tags. The emitted chunk.json stays byte-compatible with what `griff manifest` reads. 17/17 web tests pass; wasm32 release build green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new ChangesTechnique auto-derivation into ChunkMeta
Sequence Diagram(s)sequenceDiagram
participant Curator
participant build_chunk_meta
participant derive_techniques
participant merge_tags
participant ChunkMeta
Curator->>build_chunk_meta: inputs (track_index, chosen_tag_indices, score)
build_chunk_meta->>derive_techniques: (score, track_index)
derive_techniques-->>build_chunk_meta: DerivedTechniques {tags, names}
build_chunk_meta->>merge_tags: (chosen_tags, derived.tags)
merge_tags-->>build_chunk_meta: merged Vec<SwancoreTag>
build_chunk_meta->>ChunkMeta: techniques = derived.names, tags = merged
ChunkMeta-->>Curator: populated ChunkMeta record
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
|
@codex review Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eec7e70109
ℹ️ 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".
Codex P2 (#69): derive_techniques scanned track.voices.first(), but track_notes/technique_share (structure + complexity.technical) measure a track as a whole across every voice — so a chunk could count a secondary-voice technique in complexity.technical while its `techniques`/tags metadata omitted it. Scan every voice to match, the same voice-handling decision reached in #38. Flips the voice test to assert a secondary-voice technique IS derived. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
|
@codex review Addressed the P2 — Generated by Claude Code |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Why
Guitar Pro import already records techniques explicitly (ADR-0018), yet curation
left
ChunkMeta.techniquesempty and asked a human to re-tag by hand what thetab already states. This is the first step of corpus-curation automation: read
the techniques for free.
What
New
griff_core::technique:derive_techniques(score, track)— scans the track's primary voice(voice 0) — the same convention
structure/gesture/boundariesfollow —and returns the techniques the notation states are present:
SwancoreTags with a dedicated per-occurrence variant —hammer-on, pull-off, slide, bend, vibrato, palm-mute, natural/artificial
harmonic;
lower_snake_casetechnique names, alsorecording legato, accent, ghost, staccato, dead-note and tap (which have no
dedicated tag).
merge_tags(chosen, derived)— keeps the curator's order, then appendsany derived tag not already present. Stable and idempotent, so auto-derivation
adds technique tags without overriding the curator's choices.
Wired into both chunk-assembly seams — CLI
griff curate(
build_chunk_meta) and the web capture tool (build_chunk_meta_record):techniquesis now populated and the tags are the union of the curator's picksand the derived set. The emitted
chunk.jsonstays byte-compatible with whatgriff manifestreads.Deliberately out of scope
Presence-only — no thresholds or heuristics — so it stays a pure function of the
score (SPEC §6). The passage-level tags
TappingPassage/LegatoPassageareabout dominance, not presence, so they are intentionally not derived here
(a single legato slur is not a legato passage). Candidate for a follow-up.
Tests (strict TDD)
298aea9red →719557egreen forderive_techniques; the green step alsopins the voice-0 convention (a technique in a secondary voice is not derived),
caught from the CLI's
primary_voice_note_counttests.core/tests/technique_tags.rs: mapping, dedupe/determinism, primary-voice,merge_tagsorder + idempotency.techniquesand merges tags while keeping the curator's hand-picked tag.Validation
technique_tags; cli unit (incl. new wiring test) +curate_cmd10 unchanged; web 17/17 host tests.cargo clippy --workspace --all-targets -- -D warnings(incl.nursery)clean; web crate clippy clean; wasm32-unknown-unknown release build green.
🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit