Skip to content

Commit 2ff9189

Browse files
derek73claude
andcommitted
Report both branches of every fork the parser reports on
Option 2 from the review. Three silent forks now report, and the documented claim that made them defects is corrected. Renamed SUFFIX_OR_FAMILY -> SUFFIX_OR_NAME. The old name asserted a role that is only right under one of the three name orders: FAMILY_FIRST puts the declined reading in GIVEN, and the roman-numeral and comma forks decline a MIDDLE. I had already fixed the detail string for this reason; the kind name had the same bug. Generalizing it also means one kind covers every suffix-versus-name-part fork instead of accreting one per cause. Newly reported: - The trailing roman numeral. "John Smith V" takes V as a suffix where "John Smith B" makes B the family name -- V/X/I are ordinary middle initials, so that is a call, not a fact. - PARTICLE_OR_GIVEN's missing branch. "Van Johnson" reported that Van was read as a given name; "Dr. Van Johnson" made the OPPOSITE call silently, because a leading title shifts Van off index 0, the prefix chain claims it, and that branch is taken in _group while the emitter lived in _assign. Both stages now report the side they decide. The group emitter records at the merge site rather than matching the merged shape -- the first attempt keyed on "multi-token piece whose head is particle-ambiguous" and fired on "Joao da Silva do Amaral de Souza" (mid-name 'do', no fork) and on the Arabic kunya (a bound-given merge, which becomes the GIVEN name). Same decision-not-token lesson, one level in. Docs corrected: concepts.rst and the AmbiguityKind docstring both said an empty ambiguities means the parse faced no fork. That was a universal claim about the whole parser made on the evidence of three emitters, and these findings falsify it. Now: an empty tuple means none of the LISTED forks came up, coverage grows over releases, and a non-empty tuple is the signal -- an empty one is not a guarantee. AGENTS.md gains the cross-stage rule: if a fork's branches are taken in different stages, each needs the emitter, the ownership map must list ambiguities for each, and it passes vacuously until a case row exercises the path. Verified: 486-name corpus, 2 intentional diffs, 0 unexplained. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 81ea9c1 commit 2ff9189

11 files changed

Lines changed: 133 additions & 35 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ The 2.0 rewrite lands as underscore-private modules alongside the v1 code. These
119119
- **Canonical field order** — the seven roles in `Role` enum declaration order, defined once and derived everywhere (properties, `as_dict`, reprs, `comparison_key`). Never restate the order literally.
120120
- **Method organization**, fixed section order in every class: fields + `__post_init__` validation → alternative constructors → dunders (construction/equality → protocol → operators) → properties → public methods by concern (access → editing → comparison → rendering delegates) → private helpers last, except a helper serving exactly one section may sit at that section's head. Sanctioned deviation, facade layer only: `HumanName` and the shim `Constants` organize by v1 concern groups (`# -- render defaults --`, `# -- config / parsing --`, `# -- fields --`, ..., dunders and pickle last) — the classes mirror v1's own surface and die in 3.0; the canonical order still binds every core type.
121121
- **Validation is eager and fail-loud**: every `raise` states the offending value, the expected form, and the fix. Exception taxonomy: wrong type — including wrong element type inside a collection, bare `str` where an iterable of strings is expected, or a `Mapping` where a plain iterable is expected — raises `TypeError`; well-typed but unacceptable values raise `ValueError`; failed enum lookups stay `ValueError` for any input (stdlib `EnumType` precedent).
122-
- **Ambiguities are emitted at the DECISION site**: an `Ambiguity` records a fork the parse had to call, not a token that sits in an ambiguous vocabulary. Emit where the branch is taken — the trailing-suffix peel in `_assign`, the delimiter escape's follow-up in `classify` — never by scanning for a `vocab:*-ambiguous` tag. The same tagged token is a genuine fork in one position and unremarkable in another (`do` mid-name in "Joao da Silva do Amaral de Souza" chooses nothing), and structure often settles the question before it arises, which is why `PARTICLE_OR_GIVEN` is deliberately not emitted on the `FAMILY_COMMA` path and `SUFFIX_OR_FAMILY` is not emitted for "Ma, Jack". The decision site also has the token index and the detail text in hand, which the tag scan would have to reconstruct. Report BOTH directions of a two-way fork — "John Smith MA" (read as a suffix) and "Jack MA" (read as the family name) are equally guesses. Every kind needs a trigger in `tests/v2/test_contracts.py::_AMBIGUITY_TRIGGERS` (an explicit `None`, strict-xfail, while reserved), and case-table rows pin expected kinds exactly, so a new emitter shows up in both immediately.
122+
- **Ambiguities are emitted at the DECISION site**: an `Ambiguity` records a fork the parse had to call, not a token that sits in an ambiguous vocabulary. Emit where the branch is taken — the trailing-suffix peel in `_assign`, the delimiter escape's follow-up in `classify` — never by scanning for a `vocab:*-ambiguous` tag. The same tagged token is a genuine fork in one position and unremarkable in another (`do` mid-name in "Joao da Silva do Amaral de Souza" chooses nothing), and structure often settles the question before it arises, which is why `PARTICLE_OR_GIVEN` is deliberately not emitted on the `FAMILY_COMMA` path and `SUFFIX_OR_NAME` is not emitted for "Ma, Jack". The decision site also has the token index and the detail text in hand, which the tag scan would have to reconstruct. **If a fork's two branches are taken in DIFFERENT stages, every one of them needs the emitter** -- `PARTICLE_OR_GIVEN` is decided in `_assign` when the ambiguous particle stays a lone leading piece and in `_group` when a title shifts it off index 0 and the prefix chain claims it, so both report; for two years only the first did. The stage-ownership map in `tests/v2/pipeline/test_state.py` must list `ambiguities` for each such stage, and it passes vacuously until a case row exercises the path, so add the row too. Report BOTH directions of a two-way fork — "John Smith MA" (read as a suffix) and "Jack MA" (read as the family name) are equally guesses. Every kind needs a trigger in `tests/v2/test_contracts.py::_AMBIGUITY_TRIGGERS` (an explicit `None`, strict-xfail, while reserved), and case-table rows pin expected kinds exactly, so a new emitter shows up in both immediately.
123123
- **Invariants guard harm, not no-ops**: add a constructor check when violating it produces a *wrong parse*, not when it produces *nothing*. A false positive costs a working configuration; a true positive on an inert condition costs the user nothing, so that trade is never worth taking. `suffix_acronyms_ambiguous ∩ suffix_words` is guarded because the overlap loses a family name; `given_name_titles` is not, because an unreachable entry is simply never consulted (see Gotchas). Before adding one, construct the config it forbids and check what actually breaks.
124124
- **The shim TRANSLATES; it never raises on a config v1 accepted, and never silently changes the parse**: `Constants._snapshot()` is a translation boundary between v1's model and v2's invariants, and every transformation there carries its v1-reachability argument in a comment. Four exist today — `first_name_titles` re-folded per word (v1 joins-then-`lc`, v2 normalizes-then-joins), `suffix_acronyms_ambiguous ∩ acronyms` (a provable no-op), `suffix_words − ambiguous` (v1 already accepts the word via the acronym branch, so the addition is inert there), and `particles_ambiguous ∪ (bound ∩ particles)` (a pinned deviation, `test_bound_never_given_prefix_deviates_on_two_pieces`). When a v1 config cannot satisfy a v2 invariant, work out what v1 actually *does* with it — usually nothing — and reproduce that; weakening the invariant or letting the raise through are both wrong. **Test the case the translation decides**, not one where both branches agree: a test using an input v1 parses identically with and without the config pins nothing.
125125
- **Reprs are bounded**: render which fields deviate from a named baseline and by how much, never contents (`Lexicon(default + titles: +2)`).

docs/concepts.rst

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,8 +161,15 @@ de Souza"`` it sits mid-name, where nothing has to choose between
161161
readings — so nothing is recorded. A comma can settle the question
162162
before it arises, too: ``"Ma, Jack"`` fixes the family name, so the
163163
credential reading never comes up, while ``"John Smith MA"`` has to
164-
call it and says so. An empty ``ambiguities`` means the parse faced no
165-
fork, not that every word in it was unambiguous.
164+
call it and says so.
165+
166+
So an empty ``ambiguities`` does not mean every word was unambiguous —
167+
it means none of the forks the parser *reports on* came up. The kinds
168+
it reports are listed in :class:`~nameparser.AmbiguityKind`, and they
169+
are the ones where both readings are genuinely common in real names.
170+
Coverage grows over releases; a name that reports nothing today may
171+
report something later, so treat a non-empty ``ambiguities`` as a
172+
signal worth acting on rather than an empty one as a guarantee.
166173

167174
:class:`Tokens <nameparser.Token>` also carry tags — a second, independent label alongside their
168175
role, recording how a token was classified rather than what part of

docs/release_log.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ Release Log
2424
- Add ``PatronymicRule`` with the members ``EAST_SLAVIC`` and ``TURKIC``. v1's single ``patronymic_name_order`` flag enabled both detectors at once; ``Policy(patronymic_rules=...)`` lets you enable either one alone
2525
- Add ``PolicyPatch`` and the ``UNSET`` sentinel for partial policy deltas that compose -- set-valued fields union, scalar fields override with later winning. This is the mechanism locale packs are built from, and ``UNSET`` is only needed when you must distinguish "not set" from a real ``False`` or ``None``
2626
- Add ``Token``, ``Span`` and ``Role``: every field is backed by tokens carrying exact ``(start, end)`` offsets into the original string, reachable with ``tokens_for(Role.GIVEN)``. This replaces v1's ``*_list`` attributes and makes it possible to highlight or re-slice the input the parse came from
27-
- Add ``Ambiguity`` and the ``AmbiguityKind`` enum, so a parse reports what it had to guess at instead of guessing silently. The kinds emitted today are ``particle-or-given``, ``suffix-or-family``, ``suffix-or-nickname``, ``unbalanced-delimiter`` and ``comma-structure``; ``order`` is reserved and not yet emitted. The two suffix kinds cover the post-nominals that are also ordinary words: ``"John Smith MA"`` reports that ``MA`` was read as a credential rather than a surname, and ``"JEFFREY (JD) BRICKEN"`` that the delimited ``JD`` was read as a nickname rather than a suffix. A reading the vocabulary settles on its own — ``"John Smith M.A."``, ``"Andrew Perkins (MBA)"`` — is not a guess and reports nothing
27+
- Add ``Ambiguity`` and the ``AmbiguityKind`` enum, so a parse reports what it had to guess at instead of guessing silently. The kinds emitted today are ``particle-or-given``, ``suffix-or-name``, ``suffix-or-nickname``, ``unbalanced-delimiter`` and ``comma-structure``; ``order`` is reserved and not yet emitted. The two suffix kinds cover the post-nominals that are also ordinary words: ``"John Smith MA"`` reports that ``MA`` was read as a credential rather than a surname, and ``"JEFFREY (JD) BRICKEN"`` that the delimited ``JD`` was read as a nickname rather than a suffix. A reading the vocabulary settles on its own — ``"John Smith M.A."``, ``"Andrew Perkins (MBA)"`` — is not a guess and reports nothing
2828
- Add ``ParsedName`` output and comparison methods: ``render(spec)``, ``initials()``, ``capitalized()`` (which returns a new value rather than mutating in place), ``as_dict()``, ``replace(**fields)``, ``matches()`` and ``comparison_key()``
2929
- Add ``Locale``, the public pack type. Writing your own needs no registration -- construct a ``Locale`` and pass it to ``parser_for()``
3030
- Ship a fully typed public API (PEP 561): the core modules are checked under strict mypy settings, and nameparser 2.0 has no runtime dependencies

docs/usage.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ given name it stays the surname — either way the choice is recorded:
327327
>>> parse("John Smith MA").suffix
328328
'MA'
329329
>>> [a.kind.value for a in parse("John Smith MA").ambiguities]
330-
['suffix-or-family']
330+
['suffix-or-name']
331331
>>> parse("Jack MA").family
332332
'MA'
333333

nameparser/_pipeline/_assign.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,10 @@ def _assign_main(seg_idx: int, state: ParseState,
140140
if (k == len(rest) and k >= 2 and len(piece) == 1
141141
and _ROMAN.match(tokens[piece[0]].text)
142142
and "initial" not in tokens[pieces[rest[k - 2]][0]].tags):
143+
# a trailing single letter is a name part unless it happens
144+
# to be a roman numeral -- and V/X/I are ordinary middle
145+
# initials, so taking it as a suffix is a call, not a fact
146+
ambiguous_picks.append(piece)
143147
k -= 1
144148
continue
145149
# A bare ambiguous acronym ("MA", not "M.A.") is a credential
@@ -182,7 +186,7 @@ def _assign_main(seg_idx: int, state: ParseState,
182186
("a suffix", "a name part") if token.role is Role.SUFFIX
183187
else (f"a {role} name", "a post-nominal"))
184188
ambiguities.append(PendingAmbiguity(
185-
AmbiguityKind.SUFFIX_OR_FAMILY,
189+
AmbiguityKind.SUFFIX_OR_NAME,
186190
f"{token.text!r} written without periods is both a "
187191
f"post-nominal and an ordinary name; read as {taken} "
188192
f"rather than {declined}",

nameparser/_pipeline/_group.py

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,13 @@
2323
from collections.abc import Sequence, Set
2424
from enum import IntEnum
2525

26-
from nameparser._pipeline._state import ParseState, Structure, WorkToken
26+
from nameparser._pipeline._state import (
27+
ParseState, PendingAmbiguity, Structure, WorkToken,
28+
)
2729
from nameparser._pipeline._vocab import D as _D
2830
from nameparser._pipeline._vocab import PH as _PH
2931
from nameparser._pipeline._vocab import delimiter_cores
30-
from nameparser._types import Role
32+
from nameparser._types import AmbiguityKind, Role
3133

3234
# the credential-pair regexes live in _vocab (shared with segment)
3335

@@ -87,9 +89,13 @@ def _is_rootname(piece: Sequence[int], ptags: Set[str],
8789
def _group_segment(seg: tuple[int, ...], additional: int,
8890
tokens: Sequence[WorkToken],
8991
bound_join: BoundJoin = BoundJoin.STRICT,
90-
) -> tuple[list[Piece], list[set[str]]]:
92+
) -> tuple[list[Piece], list[set[str]], list[int]]:
9193
pieces: list[Piece] = [[i] for i in seg]
9294
ptags: list[set[str]] = [set() for _ in seg]
95+
# token indices where an ambiguous particle was chained into the
96+
# family name from the position that would otherwise be the given
97+
# name -- the other branch of the fork _assign reports (see group())
98+
particle_forks: list[int] = []
9399

94100
def title(k: int) -> bool:
95101
return _is_title_piece(pieces[k], ptags[k], tokens)
@@ -168,6 +174,10 @@ def merge(lo: int, hi: int, add: Set[str] = frozenset(),
168174
j += 1
169175
while j < len(pieces) and not prefix(j) and not suffix(j):
170176
j += 1
177+
if (all(title(x) for x in range(k))
178+
and "vocab:particle-ambiguous"
179+
in tokens[pieces[k][0]].tags):
180+
particle_forks.append(pieces[k][0])
171181
merge(k, j, drop={"prefix"})
172182
k += 1
173183
# bound given names: the first non-title piece joins the next
@@ -185,12 +195,13 @@ def merge(lo: int, hi: int, add: Set[str] = frozenset(),
185195
if not title(k) and not suffix(k))
186196
if non_suffix >= bound_join:
187197
merge(first_name_k, first_name_k + 2)
188-
return pieces, ptags
198+
return pieces, ptags, particle_forks
189199

190200

191201
def group(state: ParseState) -> ParseState:
192202
tokens = list(state.tokens)
193203
dropped = list(state.dropped)
204+
ambiguities = list(state.ambiguities)
194205
all_pieces: list[tuple[tuple[int, ...], ...]] = []
195206
all_ptags: list[tuple[frozenset[str], ...]] = []
196207
# v1 parity: additional_parts_count=1 applies only to FAMILY_COMMA
@@ -209,8 +220,8 @@ def group(state: ParseState) -> ParseState:
209220
else BoundJoin.DISABLED)
210221
else:
211222
bound_join = BoundJoin.STRICT
212-
pieces, ptags = _group_segment(seg, additional, tokens,
213-
bound_join)
223+
pieces, ptags, particle_forks = _group_segment(
224+
seg, additional, tokens, bound_join)
214225
if tail_start is not None and seg_idx >= tail_start:
215226
# v1 renders each tail COMMA SEGMENT as one suffix entry
216227
# ('Smith, V MD' -> suffix 'V MD'); a delimiter core inside
@@ -271,6 +282,23 @@ def group(state: ParseState) -> ParseState:
271282
ptags[m:j] = []
272283
all_pieces.append(tuple(tuple(p) for p in pieces))
273284
all_ptags.append(tuple(frozenset(t) for t in ptags))
285+
# The other half of PARTICLE_OR_GIVEN. _assign reports the fork
286+
# when an ambiguous particle stays a lone leading piece ("Van
287+
# Johnson" -> given). The prefix chain above takes the opposite
288+
# branch whenever a title shifts it off index 0 ("Dr. Van
289+
# Johnson" -> family "Van Johnson"), and that branch lives in
290+
# this stage, so it has to report here -- a fork whose two sides
291+
# are decided in different stages needs an emitter in each.
292+
# Suppressed after a family comma for the same reason _assign
293+
# suppresses it there: the family name is already fixed.
294+
if not family_comma:
295+
for i in particle_forks:
296+
ambiguities.append(PendingAmbiguity(
297+
AmbiguityKind.PARTICLE_OR_GIVEN,
298+
f"{tokens[i].text!r} was chained into the family "
299+
f"name; it is also a given name in other names",
300+
(i,)))
274301
return dataclasses.replace(
275302
state, tokens=tuple(tokens), pieces=tuple(all_pieces),
276-
piece_tags=tuple(all_ptags), dropped=tuple(dropped))
303+
piece_tags=tuple(all_ptags), dropped=tuple(dropped),
304+
ambiguities=tuple(ambiguities))

nameparser/_types.py

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -228,10 +228,12 @@ class AmbiguityKind(StrEnum):
228228
existing values never change meaning.
229229
230230
A kind names a FORK THE PARSE HAD TO CALL, not a word that could be
231-
read two ways. The same token elsewhere in a name may present no
232-
choice at all and is then reported by nothing -- an empty
233-
``ambiguities`` means the parse faced no fork, not that every word
234-
was unambiguous."""
231+
read two ways: the same token elsewhere in a name may present no
232+
choice at all and is then reported by nothing. An empty
233+
``ambiguities`` therefore means none of the forks listed here came
234+
up -- not that the parse was certain of everything. Coverage grows
235+
over releases, so a non-empty tuple is a signal to act on; an empty
236+
one is not a guarantee."""
235237

236238
#: Reserved: the name's field order itself is uncertain (e.g. a
237239
#: two-word name under a non-default name_order). Not yet emitted;
@@ -242,11 +244,15 @@ class AmbiguityKind(StrEnum):
242244
#: (JD) BRICKEN" keeps the nickname reading, where the
243245
#: unambiguous "(MBA)" escapes to suffix on vocabulary alone.
244246
SUFFIX_OR_NICKNAME = "suffix-or-nickname"
245-
#: An ambiguous suffix acronym written without periods, which is
246-
#: also an ordinary surname -- "John Smith MA" reads MA as a
247-
#: post-nominal because a family name remains, "Jack MA" reads it
248-
#: as the family name because none would.
249-
SUFFIX_OR_FAMILY = "suffix-or-family"
247+
#: A trailing word reads plausibly as either a post-nominal or an
248+
#: ordinary name part. Covers an ambiguous acronym written without
249+
#: periods ("John Smith MA" takes MA as a credential because a
250+
#: family name remains; "Jack MA" keeps it as the name because none
251+
#: would) and a trailing roman numeral, which is a suffix where any
252+
#: other single letter would be a name ("John Smith V" vs "John
253+
#: Smith B"). Which name part was declined depends on position and
254+
#: ``name_order``, so ``detail`` names it rather than the kind.
255+
SUFFIX_OR_NAME = "suffix-or-name"
250256
#: A leading ambiguous particle was read as a given name -- "Van
251257
#: Johnson" parses given="Van", but "Van" is also a family-name
252258
#: particle in other names.

0 commit comments

Comments
 (0)