Skip to content

Chord.Parse accepts quality characters in any order and any number: "C7M", "C57", "C77" and "C7m" all parse as C7 #346

Closed

Description

@matt-edmondson

What's wrong

Once the modifiers have been consumed, IsQualityVocabulary (Chord.cs:200-211) accepts a body made of any mix of m M - ° + Δ 5 6 7, in any order and in any count.

DetermineQuality and DetermineSeventh only read characters at particular positions. For example, IsMinor checks StartsWith('m'), and "M7" is only recognised when the two characters are adjacent. Characters those readers don't look at are dropped. This is the "dropped silently" case the #280 check was meant to close.

Reproduced

Run through the built library at 7167117:

  • C7M gives C7. 7M is the Brazilian and French spelling of maj7, so this is a wrong chord, not just ignored junk.
  • C7m gives C7.
  • C57 gives C7.
  • C77 gives C7.
  • Cmm7 gives Cm7.

Suggested fix

Make the leftover check accept only the spellings the readers actually understand:

  • an optional quality prefix: m, min, -, dim, °, aug, +, maj/Maj/M/Δ, or sus2/sus4/sus;
  • followed by at most one 6 or 7, or 5 for a power chord.

Reject anything else. Optionally, read 7M as maj7.

Acceptance criteria

  • Each input above is rejected, except that C7M may instead be read as Cmaj7.
  • The round-trip corpus and ChordTests still pass.

Activity

  1. matt-edmondson commented on Sep 30, 2026

    @matt-edmondson
    ContributorAuthor

    Triage


    Generated by Claude Code

  2. matt-edmondson commented on Sep 30, 2026

    @matt-edmondson
    ContributorAuthor

    Covered by #360, which redesigns this bug cluster as a whole. Implement it through that issue rather than individually.


    Generated by Claude Code

  3. matt-edmondson commented on Oct 26, 2026

    @matt-edmondson
    ContributorAuthor

    This was fixed by d6116b6, which only closed the first issue it named. Checked on main at 7c693f4: C57, C77, C7m and Cmm7 are rejected, and they are listed in Parse_RejectsInvalidGrammar. C7M is still accepted, because it is the documented major-seventh suffix spelling and gives Cmaj7. Closing.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions