fix(gp): GP5 chord enrichment — diagram-match gate + decoupled name/fingers (E3 follow-up)#523
Merged
Merged
Conversation
…uple name/fingers Post-merge Codex review of E3 (PR #522) flagged two GP5 edge cases: - P2: enrichment applied the diagram's name/fingers to the played template without checking the diagram described the voicing actually played. A mismatched chord label/diagram could mis-name/finger the played template, and the back-fill spread it to other strums of the same played pattern. Now gated on an exact, full-span fret-pattern match (new _chord_diagram_frets), mirroring the GP8 guard — and comparing over max(played width, num_strings) so a 7/8-string diagram can't falsely match a narrower played voicing. - P3: name and fingers back-fill were coupled (a name-only first annotation blocked a later beat's fingers). Now independent. Codex re-reviewed twice (the first match-gate trimmed extended strings; fixed by the full-span compare); final pass clean. +4 tests (mismatch-not-applied, name-then-fingers decoupled, higher-position absolute match, 7-string extended string regression). 163 GP tests pass. Follow-up to #522. Part of #334. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #522 (E3 GP chord-diagram extraction). An independent post-merge Codex review surfaced two GP5-path edge cases; this fixes both.
_chord_diagram_frets(); enrich only on an exact, full-span fret-pattern match (mirrors the GP8 guard). Compared overmax(played width, num_strings)so a 7/8-string diagram fretting an extended string can't falsely match a narrower played voicingReview
Codex re-reviewed twice: the first match-gate trimmed diagram frets beyond the played width (could still false-match on 7/8-string), fixed by the full-span compare; the final pass is clean (no P1/P2/P3).
Tests (
tests/test_gp2rs.py)+4 cases: mismatch-not-applied, name-then-fingers decoupled, higher-position absolute-fret match, and a 7-string extended-string regression.
Follow-up to #522. Part of #334.
🤖 Generated with Claude Code