Apply suffix_delimiter only at suffix-consumption sites - #206
Merged
Conversation
Previously the delimiter split ran unconditionally on every post-comma segment before the parser had decided which segment is actually a suffix group. That let it leak into first/middle-name segments in inverted format (e.g. "Doe, Mary - Kate, RN"), which was documented as a known limitation. Move the split to the three places that actually treat a segment as suffixes: the suffix-comma vs. lastname-comma format detection, and the two suffix_list += parts[...] consumption points. The detection check now expands parts[1] to correctly recognize delimiter-joined suffixes (e.g. "RN - CRNA") without ever expanding a segment that turns out to be a name.
…ents - Lock in hn.middle for the "Doe, Mary - Kate, RN" case so the stray hyphen artifact (a pre-existing tokenization quirk, reproducible without suffix_delimiter set) can't silently drift. - Add coverage for the parts[1:] loop expanding more than one comma segment, for a multi-word token on one side of the delimiter in the detection check, and for delimiter no-op parity with the no-delimiter baseline when the format isn't detected as suffix-comma. - Note in expand_suffix_delimiter's docstring that it's a no-op without a configured delimiter, and comment why detection needs the delimiter-expanded view of parts[1].
derek73
added a commit
that referenced
this pull request
Aug 24, 2026
The review round found the first draft shipped the inverse of the bug it
fixed. It let ANY piece open an entry, as the tail block always had --
safe there, because assign routes every tail piece to SUFFIX, which is
what `tail` means, and wrong off it, where a title piece routes to TITLE.
Two failures, one cause, neither visible to the gates that passed. The
`joined` tag is role-BLIND and the facade heals it for every role:
"Smith, Rev. Dr." title_list ['Rev.','Dr.'] -> ['Rev. Dr.']
"Smith Jr., Mr. Jr." suffix 'Jr., Jr.' -> 'Jr. Jr.'
The second glues a suffix backward across a comma the writer typed --
exactly what #429 exists to stop. The differential compares strings and
cannot see the first; the case table asserts the title STRING, which is
space-joined either way, and could not see it either.
Two joins that had been one, separated: WITHIN a piece the tag renders a
merged piece as one unit whatever role it holds; BETWEEN pieces it
continues an entry, and only a piece rendering into the same run may do
that. Sticky across a piece that is not in the entry, so an interleaved
title does not split its run ("Smith, MD Dr. PhD" -> 'MD PhD'); a
delimiter core still closes it.
Eight case rows and a facade test for the list views, which is the only
surface that shows the title collapse. Both regression guards verified
against a mutation copy -- they fail with the old condition restored.
Prose corrections, all measured by the reviewers:
- The round-trip claim was false AND backwards: str() of a fixed parse
is a no-comma string, which re-parses with the comma back. master was
the str-stable one. Struck from the release log and the case note.
- "one-word family comma" is not the condition -- there is no word-count
gate, so "John Smith, Jr. III" moves too (1.4.0's reading), as does a
title-led "Smith, Dr. MD PhD". Scope restated as it reads.
- The delimiter parity is #206 (021823e, "Apply suffix_delimiter only at
suffix-consumption sites"), NOT #191, the German/Dutch vocabulary PR.
Three code comments carried the error; corrected with it.
- The dormant-rule tell is #373's, and #426 the precedent for dropping a
shadowed rule -- neither #424 entry mentions it.
- "boundary example" in the entry and both ledgers: the example FIRES,
which is why the annotation came off.
- "filed rather than folded in" claimed an issue that does not exist.
C1 gains `_group.py` in `implemented:`, with the verbatim citation the
equality guard requires -- the whole-run half of the rule renders here.
Co-Authored-By: Claude Opus 5 <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.
Summary
suffix_delimiterpreviously split every post-comma segment unconditionally, before the parser decided which segment was actually a suffix group. This let it leak into first/middle-name segments in inverted format (e.g."Doe, Mary - Kate, RN"), a case that was documented inConstants.suffix_delimiter's docstring as a known limitation.are_suffixes_after_comma), and the twosuffix_list += parts[...]consumption points (suffix-comma branch'sparts[1:], lastname-comma branch'sparts[2:]).parts[1]on the delimiter to correctly recognize delimiter-joined suffixes (e.g."RN - CRNA") while never expanding a segment that turns out to be a name.test_suffix_delimiter_inverted_format_known_limitationwithtest_suffix_delimiter_inverted_format_not_misparsed, asserting the corrected parse instead of just documenting the bug.Test plan
pytest tests/test_suffixes.py -k suffix_delimiter -v— all 20 passpytest -q— 1242 passed, 4 skipped, 22 xfailed, no regressions