Read C69, Cadd11 and Cadd13 as added tones, and reject leftover text [patch] - #293
Merged
Merged
Conversation
…[patch] Only "add9" was consumed as an added tone. The bare "9", "11" or "13" left behind by "C69", "Cadd11" or "Cadd13" was then read as a stacked extension that implies a dominant seventh, and the leftover "add" was ignored, so the parse succeeded with the wrong notes. - Rewrite the unslashed six-nine "69" to "6add9", matching "6/9". - Consume "add11" and "add13" as added tones, and format a seventh-less chord's natural tensions as "addN" so they round-trip. - Fail the parse when the body still holds anything beyond the quality and seventh vocabulary once every token is consumed. Fixes #280 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PKmokmgnpkeYgp439Qbthh
…erload [patch] string.Replace(string, string, StringComparison) does not exist on netstandard2.0, so Semantics.Music failed to build there. Remove each word with the parser's own ordinal Take instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PKmokmgnpkeYgp439Qbthh
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PKmokmgnpkeYgp439Qbthh
|
This was referenced Sep 28, 2026
This was referenced Sep 28, 2026
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
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.



Fixes #280
What was wrong
Chord.TryParseconsumed onlyadd9as an added tone. The bare9/11/13left behind byC69,Cadd11orCadd13was read byApplyExtensionsas a stacked extension, which forces a dominant seventh. The leftoveraddwas ignored, so the parse succeeded with the wrong notes:C69came out asC769, andCadd11came out asC711.Change
RewriteSixNinerewrites a6directly followed by a bare9in the body to6add9. This is the same readingTryRewriteSixNinealready givesC6/9.add11/add13. These are now consumed as added tones next toadd9, so they no longer imply a seventh. When a chord has no seventh, the formatter writes each natural tension asaddN(add9,add11,add13), so these chords round-trip. Chords with a seventh format exactly as before.TryParsefails if the body holds anything besides the quality/seventh vocabulary thatDetermineQuality/DetermineSeventhread (maj,min,m,M,-,dim,°,aug,+,Δ,sus/sus2/sus4,5,6,7).Behaviour change to note
The leftover check means symbols that used to parse leniently into a different chord are now rejected. Examples are
Cadd,Cadd4,Cadd2and unknown suffixes such asCø7orC7alt. This is the strictness the issue asked for. If some of these should be supported, each needs its own token.Tests
ChordTonestable:C69,Cm69,Cadd11,Cadd13with the correct tones from the issue.Parse_UnslashedSixNine_IsTheSameChordAsTheSlashedSpellingParse_AddedEleventhAndThirteenth_DoNotImplyASeventhParse_UnrecognisedTextLeftInTheBody_FailsC69,Cm69,Cadd11,Cadd13,Cmadd11.With
Chord.csreverted, 4 of the Music tests fail. With the change, the fullSemantics.Testsuite passes: 1336 passed, 8 skipped.🤖 Generated with Claude Code
https://claude.ai/code/session_01PKmokmgnpkeYgp439Qbthh
Generated by Claude Code