refactor(group,docs): the cleanup round, and the pin the guard was missing

/simplify, four angles. Reuse and efficiency returned no changes; the
other two found real work.

The one that matters: the diff's central conjunct was UNPINNED. Dropping
`in_entry and` from the tag test left the whole suite green -- including
the guard added for exactly this -- while diverging on 1,152 of 65,725
inputs. That guard pins the sticky entry_open update, not the tag
condition. The distinguishing input needs a suffix piece FIRST and then
TWO titles, so the second title continues an entry it does not belong to:
"Smith, MD Rev. Dr." gives title_list ['Rev.','Dr.'] and the mutant gives
['Rev. Dr.'], with the title STRING identical either way. Added, and
verified to fail against the mutation copy.

The invariant this change rests on -- one comma segment is one suffix
entry -- was stated in NO rule. C1 governs routing; R1 governed word
ORDER and never mentioned separation; the only statement was a code
comment. That is a large part of why two stages disagreed about it for
years, so R1 gains the sentence and two examples, both names already in
the corpus (a third, "Smith, MD, PhD", would have moved for a #325
routing reason and been misfiled under #429).

Also from the review:

- Two case rows removed. family_comma_three_credential_entries is a
  strict kill-subset of the title-led row AND its note claimed to pin a
  latch that family_comma_title_between_credentials actually pins -- all
  three of its pieces are in the entry, so the latch never has to survive
  a gap. family_comma_title_run_does_not_join kills nothing, by its own
  note; test_facade.py holds the same input with assertions that can see
  the collapse.
- `family_comma` is redundant by invariant (segment() emits at most one
  segment for NO_COMMA; SUFFIX_COMMA short-circuits on `tail`). Kept for
  locality, now documented as unpinnable rather than left to look like
  missing coverage.
- I reintroduced #191 in my own new comment while this same PR corrects
  it elsewhere. Fixed here and at the two remaining delimiter sites.
  test_group.py:128 keeps #191 -- it is genuinely about the prefixes.
- The predicate moved below _is_suffix_piece, which it calls, restoring
  the _is_leading_title/_leading_titles pair it had split.
- The "whatever role it holds" claim is qualified: no multi-token TITLE
  piece witnesses that branch in 38,892 generated inputs.
- The docstring's regression narration trimmed to the contract sentence
  plus a pointer; the concrete account lives at the call site.

Efficiency recorded rather than optimised: _segment_holds_no_name runs
twice per family-comma parse, measured pure recomputation (408 of 506
comma-bearing literals reach both sites, 0 disagreements). Worth 1.2-2.2%
of a family-comma parse and 0% of every other. Caching needs a derived
ParseState field the sites would not otherwise share, so decisions.md
carries the number.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: derek73

docs/design/decisions.md

@@ -442,6 +442,8 @@ Excluded (MAIDEN_MARKERS, per nameparser/config/maiden_markers.py):
   SCOPE, stated as the condition actually reads rather than as the first draft described it: any family comma whose following segment holds no name word, NOT only a one-word family. `John Smith, Jr. III` renders suffix 'Jr. III' (1.4.0's reading), and a title-led segment is in too — `Smith, Dr. MD PhD` renders 'MD PhD'. The differential cannot witness the widening: of 1069 corpus names, 303 carry a structure comma, the predicate is true for 36, only 9 of those have two or more pieces in the segment, and none of the 9 combines a multi-word pre-comma name with two suffix pieces. 0 unexplained is therefore evidence about the corpus, not about the class — the case table carries it instead.
   THE REGRESSION THE REVIEW FOUND, and the reason this entry is long: the first draft let ANY piece open an entry, as the tail block always had. That is safe on a tail segment, where assign routes every piece to SUFFIX — which is what `tail` means — and wrong off it, where a title piece routes to TITLE. Two failures, one cause. The `joined` tag is role-BLIND and the facade heals it for every role, so a title opening an entry collapsed `title_list` from ['Rev.', 'Dr.'] to ['Rev. Dr.'] on `Smith, Rev. Dr.`; and where the pre-comma name left a suffix, the tag glued the next suffix BACKWARD across the writer's own comma — `Smith Jr., Mr. Jr.` rendered suffix 'Jr. Jr.', the exact inverse of the bug being fixed. Neither is visible to the differential, which compares strings, nor to the case table as it stood, which asserts the title STRING (space-joined either way). The rule that replaces it separates two joins that had been one: 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 that renders into the same run may do that. The entry is sticky across a piece that is not in it, so an interleaved title does not split the run it sits in (`Smith, MD Dr. PhD` renders 'MD PhD'); a delimiter core still closes it, which is the one thing that separates two entries.
   Scoped deliberately, and the scoping is the other decision: `tail` still gates delimiter-core dropping, which is v1 `expand_suffix_delimiter` parity (#206, commit 021823e — three code comments say #191, which is the German/Dutch vocabulary PR, and are corrected here) and a TAIL rule. The core set is threaded to the block as `seg_cores` so the widened condition cannot drop cores where `tail` says not to. Worth knowing before anyone tries to pin that: for every realistic delimiter the two spellings are indistinguishable by construction, because the core must first pass the very predicate that gates the block — the only reachable class is a policy whose delimiter core is itself suffix or title vocabulary (`Policy(extra_suffix_delimiters={" jr "})`, `Smith, MD jr`), which the case table does not enshrine.
+  Cost, measured rather than optimised away: `_segment_holds_no_name` now runs TWICE per family-comma parse, once here and once in assign, on the same pieces — verified pure recomputation, not two questions (408 of 506 comma-bearing test literals reach both sites, 0 disagreements, and off-tail `seg_cores` is empty so `kept` never filters, which is why assign sees exactly the pieces group tested). Removing the second call is worth 0.6–1.4 µs, 1.2–2.2% of a family-comma parse and 0% of every other parse (NO_COMMA and SUFFIX_COMMA make no call at all). Caching it needs a derived ParseState field the two sites would otherwise not share, so the number is recorded instead. The order is cheapest-first per 7009fcf: two bool loads and an int compare gate the call.
+  R1 gained the separator sentence with this change. That the invariant it states — one comma segment is one suffix entry — was written down in no rule is a large part of why two stages disagreed about it for years: C1 governs routing, R1 governed word ORDER and never mentioned separation, and the only statement of the rule was a code comment.
   Left alone, measured and pre-existing: the NO-COMMA path comma-joins a space-separated run too (`John Smith MD PhD` renders 'MD, PhD' at 1.4.0, 2.0.0, 2.1.0 and master alike). The fix requires FAMILY_COMMA and does not reach it; that path has no writer-drawn boundary and is a separate question, recorded here and NOT yet filed. It is load-bearing for one thing: `str()` of a fixed parse is a no-comma string, so re-parsing it re-inserts the comma — round-tripping is not stable on these shapes, and an earlier draft of the release bullet claimed it was.
   The name `Smith, MD PhD` entered the corpora as this fix's C1 example — a FIRING one, carrying no `· boundary` annotation, which marks the non-firing example a rule must have — so its ledger entries arrive with it. A fourth written for the 1.4.0 ledger EXPLAINED NOTHING and was dropped: `fix(comma-family) lone post-comma piece routes to suffix/title, not first` already claims the name there, its fields covering all three that move. That is #373's dormant-rule diagnosis doing its job, and #426 is the precedent for dropping rather than keeping a shadowed rule.
 

docs/design/rules.md

@@ -875,10 +875,17 @@ R1. Rationale: a field is a way of reading the parse, not a stored
     words — O3's fold and, since #379, P6's attached tussenvoegsel —
     which render before the rest of the family wherever they stood in
     the string.
+    Words are separated as the writer separated them. The suffix view
+    is the one place this is visible, because it is the only field
+    that can hold parts the writer comma-separated: a run of
+    post-nominals written with spaces renders with spaces, and one
+    written with commas keeps them.
       "Dr. Juan Q. Xavier de la Vega III"  →  family="de la Vega"
       "Hassan, Mohamad Ahmad Ali"  middle_as_family  →  family="Ahmad Ali Hassan"
       "Hassan, Mohamad Ahmad Ali"          →  family="Hassan"  · boundary
-    implemented: nameparser/_types.py
+      "Smith, MD PhD"                      →  suffix="MD PhD"
+      "John Smith, MD, Bart"               →  suffix="MD, Bart"
+    history: decisions.md#C1 · implemented: nameparser/_types.py
 
 R2. Rationale: callers need the surname with and without its
     particles — sorting wants "Vega", display wants "de la Vega".

nameparser/_pipeline/_group.py

@@ -102,44 +102,6 @@ def _is_leading_title(piece: Sequence[int], ptags: Set[str],
             and bool(_PERIOD_ABBREV.match(tokens[piece[0]].text)))
 
 
-def _segment_holds_no_name(pieces: Sequence[Sequence[int]],
-                           ptags: Sequence[Set[str]],
-                           tokens: Sequence[WorkToken]) -> bool:
-    """The segment is titles and suffixes only ('John Smith, Dr.',
-    'John Smith, Mr. Jr.') -- nothing in it is a name word.
-
-    The FAMILY_COMMA rule "segment 0 is wholly the family name" rests on
-    the writer having said where the family name ends. A comma followed
-    by no name word said no such thing -- 'John Smith, Dr.' is 'Dr. John
-    Smith' with the honorific moved, and 'John Smith, Mr. Jr.' the same
-    with the postnominal along -- so the pre-comma name keeps its
-    positional read instead of being merged. Uses the same
-    _is_leading_title predicate the peel does, period-abbreviation
-    inference included, so the two cannot disagree about what a title
-    is; a suffix piece counts as what it is, so a mixed run like
-    'Smith, Dr. Jr.' is a title and a postnominal, each read where it
-    stands, and never a title run 'Dr. Jr.'. An empty segment
-    ('Doe,, Jr.') holds no title to read by.
-
-    TWO callers, asking it for different reasons, and the difference
-    matters. assign uses it to decide whether the comma fixed the family
-    name (above). group's one-entry join (#429) uses it to decide
-    whether the segment is a credential run at all.
-
-    True does NOT mean "every piece is a suffix" -- the title tolerance
-    is the whole point, and a true segment can still hold pieces assign
-    routes to TITLE. A caller that renders the segment as one unit must
-    therefore ask _is_suffix_piece per piece as well; assuming otherwise
-    is what made #429's first draft collapse title_list and glue a
-    suffix across a comma the writer typed.
-    """
-    if not pieces:
-        return False
-    return all(_is_suffix_piece(pieces[k], ptags[k], tokens)
-               or _is_leading_title(pieces[k], ptags[k], tokens)
-               for k in range(len(pieces)))
-
-
 def _leading_titles(pieces: Sequence[Sequence[int]],
                     ptags: Sequence[Set[str]],
                     tokens: Sequence[WorkToken]) -> int:
@@ -183,6 +145,44 @@ def _is_suffix_piece(piece: Sequence[int], ptags: Set[str],
     return "vocab:suffix" in tags and "initial" not in tags
 
 
+def _segment_holds_no_name(pieces: Sequence[Sequence[int]],
+                           ptags: Sequence[Set[str]],
+                           tokens: Sequence[WorkToken]) -> bool:
+    """The segment is titles and suffixes only ('John Smith, Dr.',
+    'John Smith, Mr. Jr.') -- nothing in it is a name word.
+
+    The FAMILY_COMMA rule "segment 0 is wholly the family name" rests on
+    the writer having said where the family name ends. A comma followed
+    by no name word said no such thing -- 'John Smith, Dr.' is 'Dr. John
+    Smith' with the honorific moved, and 'John Smith, Mr. Jr.' the same
+    with the postnominal along -- so the pre-comma name keeps its
+    positional read instead of being merged. Uses the same
+    _is_leading_title predicate the peel does, period-abbreviation
+    inference included, so the two cannot disagree about what a title
+    is; a suffix piece counts as what it is, so a mixed run like
+    'Smith, Dr. Jr.' is a title and a postnominal, each read where it
+    stands, and never a title run 'Dr. Jr.'. An empty segment
+    ('Doe,, Jr.') holds no title to read by.
+
+    TWO callers, asking it for different reasons, and the difference
+    matters. assign uses it to decide whether the comma fixed the family
+    name (above). group's one-entry join (#429) uses it to decide
+    whether the segment is a credential run at all.
+
+    True does NOT mean "every piece is a suffix" -- the title tolerance
+    is the whole point, and a true segment can still hold pieces assign
+    routes to TITLE, so a caller rendering the segment as one unit must
+    ask _is_suffix_piece per piece as well. What assuming otherwise cost
+    is recorded at the one-entry join in group(), the caller that made
+    the assumption.
+    """
+    if not pieces:
+        return False
+    return all(_is_suffix_piece(pieces[k], ptags[k], tokens)
+               or _is_leading_title(pieces[k], ptags[k], tokens)
+               for k in range(len(pieces)))
+
+
 class Peel(NamedTuple):
     """What assign's trailing peel made of a walk. `names` is a count
     of positions in the caller's `rest`: rest[:names] are the name
@@ -905,6 +905,13 @@ def group(state: ParseState) -> ParseState:
         # never typed, where the full-name 'John Smith, MD PhD' has
         # rendered 'MD PhD' since 1.4.0 (#429). Ask assign's own
         # predicate, over the pieces group just built.
+        # `family_comma` is redundant by invariant and kept for
+        # locality: segment() emits at most one segment for NO_COMMA, so
+        # seg_idx == 1 already implies a comma, and under SUFFIX_COMMA
+        # tail_start is 1, so `tail` short-circuits before this. Nothing
+        # can pin it -- dropping it is an equivalent mutant over the
+        # corpora and 65,725 generated inputs -- so it is documented
+        # rather than tested.
         one_entry = tail or (
             family_comma and seg_idx == 1
             and _segment_holds_no_name(pieces, ptags, tokens))
@@ -917,7 +924,7 @@ def group(state: ParseState) -> ParseState:
             # tokens within an entry take the stable "joined" tag so
             # the suffix view space-joins them (the fix_phd mechanism).
             # Core dropping stays keyed on `tail` via seg_cores: the
-            # #191 parity is a TAIL rule, and the one-entry join is the
+            # #206 parity is a TAIL rule, and the one-entry join is the
             # only half that follows assign's content read.
             entry_open = False
             kept: list[int] = []
@@ -932,8 +939,11 @@ def group(state: ParseState) -> ParseState:
                 kept.append(k)
                 # Two different joins, and conflating them is what a
                 # widened condition gets wrong. WITHIN a piece (pos > 0)
-                # the tag renders a merged piece as one unit, whatever
-                # role it holds. BETWEEN pieces it continues an ENTRY,
+                # the tag renders a merged piece as one unit; the branch
+                # is written role-blind because the merge is (the ph-d
+                # pair reaches GIVEN as one element), though no
+                # multi-token TITLE piece witnesses it -- none turned up
+                # in 38,892 generated family-comma inputs. BETWEEN pieces it continues an ENTRY,
                 # and only pieces that render into the same run may do
                 # that. On a tail segment every kept piece does -- that
                 # is what `tail` means -- but off it assign routes piece

tests/v2/cases.py

@@ -582,7 +582,7 @@ def __post_init__(self) -> None:
     Case("suffix_delimiter_tail_segment", "Doe, John, RN - CRNA",
          {"given": "John", "family": "Doe", "suffix": "RN, CRNA"},
          policy=_SD,
-         notes="v1 suffix_delimiter parity (#191): the delimiter token "
+         notes="v1 suffix_delimiter parity (#206): the delimiter token "
                "is dropped from consumed tail segments (pinned live "
                "2026-07-16)"),
     Case("suffix_delimiter_detection", "Doe, John RN - CRNA",
@@ -1647,24 +1647,6 @@ def __post_init__(self) -> None:
                "'Jr. Jr.', the exact inverse of the bug #429 fixes. "
                "Only a piece that renders into the same run may "
                "continue an entry"),
-    Case("family_comma_title_run_does_not_join", "Smith, Rev. Dr.",
-         {"title": "Rev. Dr.", "family": "Smith"},
-         notes="the same regression seen through the other view: the "
-               "'joined' tag is role-blind and the facade heals it for "
-               "EVERY role, so a title piece opening an entry "
-               "collapsed title_list from ['Rev.', 'Dr.'] to "
-               "['Rev. Dr.']. The title string is space-joined either "
-               "way, so THIS row cannot see the collapse and does not "
-               "guard it -- test_facade.py's "
-               "test_the_joined_tag_never_reaches_a_title asserts the "
-               "list views and does. Kept here for the shape; the "
-               "differential compares strings and is blind to the "
-               "whole class"),
-    Case("family_comma_three_credential_entries", "Smith, Ph. D. Jr. MD",
-         {"family": "Smith", "suffix": "Ph. D. Jr. MD"},
-         classification="fix(#429)",
-         notes="the run does not stop at two: the entry latches rather "
-               "than being reset per piece"),
     Case("family_comma_written_commas_are_kept", "Smith, MD, PhD",
          {"family": "Smith", "suffix": "MD, PhD"},
          notes="the negative control for #429, and the distinction the "

tests/v2/pipeline/test_group.py

@@ -436,7 +436,7 @@ def test_the_connective_carveout_counts_the_surviving_name() -> None:
 
 
 def test_a_delimiter_core_in_a_suffix_tail_is_not_maiden_text() -> None:
-    """A tail segment drops its delimiter cores (#191) and the marker
+    """A tail segment drops its delimiter cores (#206) and the marker
     takes what is left, in that order -- the order group() had before
     the marker pass moved ahead of the joins. A core is not a word the
     marker can take: it is structure, like the marker itself."""

tests/v2/test_facade.py

@@ -207,6 +207,17 @@ def test_the_joined_tag_never_reaches_a_title(  # #429 regression guard
     k = HumanName("Smith, MD PhD")
     assert k.suffix_list == ["MD PhD"]        # ONE element
 
+    # And the pin for the TAG CONDITION itself, which the three above
+    # miss: they all pin the sticky entry_open update, and the whole
+    # suite stays green with `in_entry and` dropped from the tag test.
+    # This needs a suffix piece FIRST -- opening the entry legitimately
+    # -- and then TWO titles, so the second title is a continuation of
+    # an entry it does not belong to.
+    j = HumanName("Smith, MD Rev. Dr.")
+    assert j.suffix == "MD"
+    assert j.title == "Rev. Dr."             # identical either way
+    assert j.title_list == ["Rev.", "Dr."]   # the mutant gives ['Rev. Dr.']
+
 
 def test_str_uses_string_format_with_v1_cleanup() -> None:
     n = HumanName("Dr. Juan de la Vega III")