feat: auto-derive chord-quality tags from chord voicings (#75)

[PhysShell]

Adds a presence-only chord-quality deriver (harmony::derive_harmony) that reads each chord group's pitch-class set and tags the voicing the tab already spells out — the next of #75's auto-derivation asks after let_ring. The tags existed in the taxonomy but were curator-only.

What

• power_chord — a bare perfect-fifth dyad (octave doublings collapse by pitch class)
• maj7 / min7 / sus2 / add9 — exact root-relative pitch-class templates, tried from every chord tone so inversions still tag their quality
• Wired into build_chunk_meta: derived tags merge additively over the curator's choices, the same treatment the derived technique tags get
• Presence-only and threshold-free — a pure function of the score (SPEC §6), mirroring derive_techniques

Deliberately deferred (see docs/decisions.log.md)

• slash_chord — its common case (G/B, a plain triad over a non-root bass) is not expressible with the seventh/sus/add templates; it needs its own bass-vs-root pass
• Arpeggiated / cross-voice voicings and extended/altered chords go unclassified for now (under-tag, never mis-tag)

No SCHEMA_VERSION bump — additive tag, per the keep-v7 policy from #87.

A note on #75's premise

While scoping this I confirmed the issue's "pull-off — the field exists in the guitarpro crate, just isn't mapped — easy" is mistaken: guitarpro v0.3's NoteEffect has only a single hammer: bool (format bit 0x02 = "hammer-on/pull-off"), already mapped to HammerOn. Telling a pull-off from a hammer-on needs pitch-direction inference against the same-string neighbour, so it was re-scoped to this (the highest-value, most self-contained ask) instead.

TDD

red (162bcda) → green (c41f3e0) → cli wiring (1c12b9e) → docs (14e0fbb) → fmt fixup (ca4501f).

Verification

• 12 new harmony_tags tests + full workspace suite green
• cargo clippy --workspace --all-targets -- -D warnings clean
• compiles with --no-default-features (the wasm/web path)

🤖 Generated with Claude Code

https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95

---

Generated by Claude Code

Summary by CodeRabbit

• New Features
  • Automatically derives chord-quality tags (maj7, min7, sus2, add9, power chord) from track audio notation and merges them into generated metadata during both CLI and web processing.
• Tests
  • Added a new test suite covering multiple chord types, octave/voicing variations, negative cases, deterministic ordering, deduplication, voice coverage, and out-of-range track handling.
• Documentation
  • Updated the decisions log with the chord-quality auto-derivation strategy, including current limits (e.g., no slash/extended classification in the first cut).

[coderabbitai]

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: c7df4b89-0476-46a5-a626-4024ee0c6b60

📥 Commits

Reviewing files that changed from the base of the PR and between ca4501f and bf7404d.

📒 Files selected for processing (1)

• web/src/lib.rs

---

📝 Walkthrough

Walkthrough

A new harmony module is added to griff_core that derives chord-quality SwancoreTags (power chord, maj7, min7, sus2, add9) from a track's pitch-class sets using presence-only template matching. Both the CLI's build_chunk_meta and web's build_chunk_meta_record are updated to call harmony::derive_harmony and merge the results into chunk metadata tags. A comprehensive test suite and a decision log entry are included.

Changes

Chord-quality auto-derivation

Layer / File(s)	Summary
harmony module: constants, classification, and pitch helpers core/src/lib.rs, core/src/harmony.rs	Publicly exports a new harmony submodule. Defines HARMONY_TAGS canonical order and root-relative pitch-class TEMPLATES for maj7/min7/sus2/add9. Implements derive_harmony (entry point), classify_group (filters chord/strum groups), pitch_classes (extracts and dedupes pitch classes from atoms), classify (power-chord dyad detection then inversion-aware template dispatch), match_templates, interval_above, and pitch_class arithmetic helpers.
CLI build_chunk_meta: harmony tag merge cli/src/main.rs	Adds harmony to the griff_core import list, computes derived_harmony in build_chunk_meta from the selected track (empty when no track is selected), and merges harmony tags additively into ChunkMeta.tags after existing curator and technique-derived tags.
Web build_chunk_meta_record: harmony tag merge web/src/lib.rs	Adds harmony to the griff_core import list, computes derived_harmony in build_chunk_meta_record and merges it into the record's tags. Updates a test index for curator "intro" tag and adds a new test that injects a chord event group and asserts SwancoreTag::PowerChord is derived and included in the result.
Test suite and decision log core/tests/harmony_tags.rs, docs/decisions.log.md	Adds a full test module with synthetic Score/EventGroup helpers covering PowerChord (dyad and octave-doubled), Maj7, Min7, Sus2, Add9, inversion invariance, deduplication across groups, canonical taxonomy ordering, out-of-range track (empty result), multi-voice scanning, and negative cases (single note, plain major triad). Appends a 2026-06-19 decision log entry defining the presence-only derivation approach and explicit deferral of slash_chord.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

• engine: auto-derive chord-quality / structural / rhythm tags; parse pull-off, tap, let-ring #75: This PR directly implements the chord-quality auto-derivation requested in issue #75, adding harmony::derive_harmony with pitch-class-set matching for maj7/min7/sus2/add9/power_chord tags.

Possibly related PRs

• PhysShell/griff#69: Modifies the same build_chunk_meta curation flow in cli/src/main.rs by adding technique-derived tag merging — this PR extends that same merge pipeline with a second harmony-derived tag step in both CLI and web layers.

Poem

🐇 Hoppity-hop through the pitch-class set,
A root and its intervals, perfectly met!
Power chords jingle — just fifth and a skip,
Maj7 and Sus2 now join the trip.
No slash chords yet, that's a tale for later days,
But harmony tags now gleam through the haze! 🎸

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.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 'feat: auto-derive chord-quality tags from chord voicings (#75)' is specific, clear, and directly summarizes the main change—adding automatic derivation of chord-quality tags from chord voicings. It matches the primary objective of this pull request.
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: ca4501f470

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

cli/src/main.rs (1)

1073-1078: ⚡ Quick win

Add a CLI-level regression test for harmony-tag merge behavior.

build_chunk_meta now changes final tags composition, but the nearby test only asserts curator+technique outcomes. Add a case asserting derived harmony (e.g., Maj7) is preserved in the same merged tag list.

Proposed test shape

+#[test]
+fn build_chunk_meta_merges_derived_harmony_tags() {
+    // build a score with a chord-bearing group, e.g. Cmaj7 pitch classes
+    // call build_chunk_meta(..., Some(track_idx), ...)
+    // assert curator tag still present
+    // assert technique-derived tags still present when applicable
+    // assert meta.tags.contains(&SwancoreTag::Maj7)
+}

As per coding guidelines, "Write tests before implementation (TDD red-green-refactor cycle): run failing tests first, commit before touching implementation code".

🤖 Prompt for 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.

In `@cli/src/main.rs` around lines 1073 - 1078, The existing test for tag merging
in `build_chunk_meta` does not verify that derived harmony tags (such as `Maj7`)
are preserved in the final merged tag list. Add a new CLI-level regression test
case that exercises the complete merge flow where curator tags are merged with
derived technique tags via the first `technique::merge_tags` call, and then the
result is merged with `derived_harmony` via the second `technique::merge_tags`
call in `build_chunk_meta`. The test should assert that harmony tags are present
alongside curator and technique tags in the final merged result. Add the failing
test first before making any implementation changes, following the TDD
red-green-refactor cycle.

Source: Coding guidelines

🤖 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 `@core/src/harmony.rs`:
- Line 49: Split the current commit ca4501f into two separate commits with the
correct order: first commit should contain only the tests for derive_harmony
from core/tests/harmony_tags.rs, and the second commit should contain the
implementation of the derive_harmony function in core/src/harmony.rs. This
ensures tests precede the implementation as required by the coding guidelines.

---

Nitpick comments:
In `@cli/src/main.rs`:
- Around line 1073-1078: The existing test for tag merging in `build_chunk_meta`
does not verify that derived harmony tags (such as `Maj7`) are preserved in the
final merged tag list. Add a new CLI-level regression test case that exercises
the complete merge flow where curator tags are merged with derived technique
tags via the first `technique::merge_tags` call, and then the result is merged
with `derived_harmony` via the second `technique::merge_tags` call in
`build_chunk_meta`. The test should assert that harmony tags are present
alongside curator and technique tags in the final merged result. Add the failing
test first before making any implementation changes, following the TDD
red-green-refactor cycle.

🪄 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: 3c7fbb61-42e6-4eea-8c7f-6dbac7dc6d2f

📥 Commits

Reviewing files that changed from the base of the PR and between 1fcf816 and ca4501f.

📒 Files selected for processing (5)

• cli/src/main.rs
• core/src/harmony.rs
• core/src/lib.rs
• core/tests/harmony_tags.rs
• docs/decisions.log.md