nameparser 2.0 implementation#288
Draft
derek73 wants to merge 273 commits into
Draft
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds tests/v2/test_layering.py to mechanically enforce the conventions
doc's import layering and the public v2 export surface. Appends the v2
core types (Span, Role, Token, Ambiguity, AmbiguityKind, ParsedName,
Lexicon, Policy, PolicyPatch, PatronymicRule, UNSET, GIVEN_FIRST,
FAMILY_FIRST, FAMILY_FIRST_GIVEN_LAST, Locale) to nameparser/__init__.py
alongside the existing v1 HumanName export.
Turns on component-flag mypy strictness for the four v2 modules and
check_untyped_defs for tests/v2, then fixes the resulting test-only
type mismatches by using the precise constructed types (Span(...),
frozenset({...}), tuple-of-pairs) where the literal type didn't matter
to the test, and adding narrow # type: ignore[arg-type] comments only
where a test deliberately exercises runtime coercion/validation of an
intentionally mismatched static type (e.g. Token span/Ambiguity kind
coercion tests, PatronymicRule string coercion, dict aliasing test).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pickle.dumps(Lexicon.default()) raised TypeError because the default slots-dataclass pickle path serializes the _cap_map MappingProxyType. Ship every other slot and rebuild the proxy from the canonical capitalization_exceptions tuple on load. Parser (Plan 3) is picklable by construction per the core spec, and a Parser holds a Lexicon. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
requires-python is >=3.11 on this branch, so the 3.10 job can no longer install the package (uv sync either fails or silently substitutes a managed 3.11). The rest of #257 (typing_extensions, classifiers) stays tracked on the issue. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With requires-python at >=3.11, Self imports directly from typing and the conditional dependency can never activate; ruff (UP035/UP036) flags the dead version block once the floor is raised. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The v1 suite is fully annotated (since #250 put tests/ under mypy), so pyproject carries no per-file ANN ignores; the new tests/v2 modules must be annotated too or 'ruff check' fails in CI. Also drops an unused import ruff flagged (F401). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A bare startswith('nameparser._types') would also admit a future
sibling like nameparser._types_helpers. Entries ending in '.' stay
pure prefixes (subpackage contents); anything else now means that
exact module or its submodules. Also carries this file's ANN
annotations.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Nothing tied the 'particle' string hard-coded in _text_for callers to the published STABLE_TAGS constant; assert the linkage in the derived-view test until Plan 3's tag-emission contract tests land. Also carries this file's ANN annotations. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Lexicon(titles={'Dr.': 'Doctor'}) silently kept only the keys -- the
lone quiet coercion on an otherwise fail-loud surface, and a dict here
almost always means the field was confused with
capitalization_exceptions.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
repr(Lexicon.empty().add(titles={'zqx'})) rendered as default() plus
ten '-N' deltas -- technically bounded, but the wrong story for the
documented build-from-empty() power-user path. Compare against both
named constructors and render the smaller diff.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pure move: constructors ahead of dunders, __or__ grouped with the dunders, then properties, then the editing methods (with _edit at the head of its section per the documented exception). No behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wrong type (including wrong element type in a collection, bare str for an iterable-of-strings field, Mapping for a plain iterable) raises TypeError; well-typed but unacceptable values (empty required strings, inverted spans, unknown enum values, subset violations) raise ValueError. Matches the boundary the generated dataclass __init__ already draws -- unknown kwargs are TypeError natively -- so the constructors can never present an all-ValueError contract anyway. Mixed checks (Token.text, Ambiguity.detail, Locale.code, delimiter pairs, extra_suffix_delimiters entries) split into a type check and a value check. Enum lookups stay ValueError for any element, matching stdlib EnumType.__call__; non-iterable patronymic_rules now surfaces its natural TypeError instead of being converted. Rule recorded in the conventions doc §6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
maiden_markers was the only Lexicon.default() field fed by an inline literal instead of a nameparser/config data module. Add config/maiden_markers.py with the #274 candidate set: French, German, Dutch, Czech/Slovak, and Russian markers, both genders and both e/yo spellings (casefold does not fold them). Non-colliding Cyrillic entries belong in the default lexicon per the locales design's sorting rule. Deliberately absent: 'z domu' (two-token marker, pending the pipeline's multi-token matching decision) and Scandinavian 'f.' (collides with the initial F). The 1.x parser does not read the module. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The issue's veto covered only the abbreviation 'f.' (collides with the initial F); the full participles født (da/nb), fødd (nn), and född (sv) cannot appear as name tokens and are the standard convention in running text. No ASCII variants -- dropping the diacritic is not standard practice in Scandinavian text, unlike nee in English. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The enforceable subset of the (untracked) conventions doc -- module layout, layering, canonical field order, method organization, exception taxonomy, bounded reprs, typing/doctest posture, pickling, the one-global rule, and tests/v2 conventions -- now has a tracked home the v1 sections don't cover, including this week's two amendments (section-head helpers, TypeError/ValueError split). Establishes the same-commit rule: convention changes update this section in the commit that makes them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
tags was the one collection input on the v2 surface without the bare-string/mapping/element-type guards: tags='particle' silently became its character set and every STABLE_TAGS-based derived view misclassified with no error. role was the one enum field without coercion; a raw 'given' string broke 'role is Role.GIVEN' identity checks silently. role now mirrors Ambiguity.kind: coerce the string form, enriched ValueError naming valid roles on any failed lookup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The four bool flags were the only unvalidated Policy fields: truthy strings like 'no' stored fine and behaved as True, silently inverting the caller's intent. And the patronymic-rule try wrapped the whole iteration, so a ValueError raised inside a caller's generator was rewritten as 'unknown patronymic rule' with the real traceback erased by 'from None'. Materialize first (the _normset pattern), convert per-element, and name the offending entry in the error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Union fields were already canonicalized at patch construction, but a list name_order stored as-is -- making the patch and any Locale holding it unhashable, with the failure surfacing only when a locale became a dict key. Policy re-coerces at apply time, so the patch was the one container in the chain that could silently carry the list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A bare string or a mis-shaped entry leaked raw unpack errors ('not
enough values to unpack') naming neither the field nor the fix -- and a
2-char string entry like 'ab' unpacked silently into {'a': 'b'}. All
three now raise the taxonomy's TypeError with the offending value and
expected form.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
__setstate__ accepted any state dict: a pickle from an older or newer Lexicon (field added, renamed) loaded fine and failed at the first attribute read, far from the unpickle site with no hint the cause was a stale pickle. Check the field-name set at load and name the missing/unexpected fields. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
'bishop' is a v1 TITLE, not a suffix word, so removing it from suffix_words asserted nothing -- the test kept passing even if remove() were a no-op. Remove an entry that is actually present and assert the precondition first. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- apply_patch revalidates deferred patch values (PolicyPatch documents lazy validation; nothing asserted the apply-time failure) - all four set-valued PolicyPatch fields declare union composition (apply_patch is metadata-driven; dropping one silently flips locale layering from union to override) - pickle round-trips for Token/Ambiguity/ParsedName/Policy/PolicyPatch, pinning that UNSET survives unpickling BY IDENTITY -- apply_patch gates on 'is UNSET', which only works because Enum members unpickle to the same object - _normalize is casefold + all-periods stripping, stricter than v1's lc() (lower + edge periods only) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three 'added in a later task' notes were false at HEAD; the _VOCAB_FIELDS comment claimed __or__ excludes capitalization_exceptions (it merges them right-biased); the _lexicon module docstring's data- module list omitted maiden_markers (now points at _default_lexicon's imports, which cannot rot); _normalize claimed equivalence with v1's lc() (lc lowers and trims only edge periods); the maiden_markers docstring omitted the masculine Russian forms it ships; replace() did not document that it drops ambiguities referencing replaced tokens; and AGENTS.md overstated 'mypy strict' (per-module strict is not valid mypy config) and the one-test-module-per-source rule. Also notes the layering table's config prefix enforces package granularity only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
'.', '', or whitespace is a data bug (stray split artifact, empty CSV cell) on the primary customization surface; silently dropping it also meant a future data-module typo would vanish instead of failing CI. One rule for vocab fields AND capitalization-exception keys -- the previous behavior dropped both silently (keys deliberately, vocab undocumented). Raising now and relaxing later is compatible; the reverse is a break. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Span(0,2) + Span(3,4) inherited tuple concatenation, producing a 4-tuple -- the natural but wrong spelling of 'covering span' that the pipeline's join stage will tempt. A real cover() operation ships with that consumer; blocking + now is compatible, blocking it after 2.0.0 would be a break. Also reject bools in span coordinates (bool is an int subclass; (False, True) is a leaked comparison result, not a span). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codes become registry keys the moment parser_for and third-party packs exist; every character accepted today is supported forever. The pin matches the shipped ru/tr_az convention and deliberately allows one separator only -- accepting '-' as well would make tr-az and tr_az distinct keys. Relaxing later is compatible; tightening later breaks published packs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Token.role and Ambiguity.kind carried byte-parallel coerce-or-raise blocks that had to be kept in sync by hand; both message shapes are pinned by tests. One parametrized module-private helper, two one-line call sites, identical messages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The module's convention is that collection normalization lives in a module-level _norm* helper the constructor calls; capitalization exceptions were the one field whose ~40-line validation body sat inlined in __post_init__, mixing orchestration with one field's rules. Pure move -- messages and behavior unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The [a-z0-9_]+ fullmatch landed after the whitespace check and fully absorbs it: all-whitespace input already fails the strip() check, and interior whitespace fails the charset with an accurate message. The empty and lowercase pre-checks stay -- they guard the common mistakes with materially friendlier messages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
d8feb3e added given_name_titles <= titles to _SUBSET_FIELDS. That is the wrong shape for the field: _post_rules space-joins a multi-token title run and looks the JOINED string up, so "grand duke" is a legitimate entry whose WORDS are titles while the phrase itself is not. The invariant made multi-word honorifics inexpressible in the 2.0 API, and the matching shim intersection silently dropped them, reclassifying the given name: Constants: titles += {grand, duke}, first_name_titles += {grand duke} "Grand Duke John" 1.4.0: first='John' d8feb3e: last='John' Check the words instead. The original footgun -- add(given_name_titles= {"sheikh"}) doing nothing -- still raises, because 'sheikh'.split() is not in titles either. The shim now drops only entries with a non-title word, which are the ones v1 could not reach. Two more fixes to the same validation block: - _SUBSET_FIELDS carried one rationale ("an orphan is never consulted") for all its pairs, and it was false for both survivors: an orphan particles_ambiguous entry makes _assign emit a spurious ambiguity, and an orphan suffix_acronyms_ambiguous entry makes _vocab treat the word as a period-gated suffix. Each pair now carries its own reason, and the reason appears in the error. - The remedy was "Add them to {base} as well", which inverts the intent of a caller who was REMOVING from the base -- telling them to add back what they just removed. Now names both directions. And adds the invariant that guard should have been: an ambiguous suffix acronym must not also be a suffix word. suffix_as_written ORs the two branches, so the word membership bypasses the period gate the ambiguous set exists to impose -- add(suffix_words={"ma"}) was accepted and made "Jack Ma" lose its family name. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
__setstate__ checked the field LAYOUT and then hand-assigned every value, so a state dict could load a Lexicon that no constructor would produce: invariants unchecked, entries unnormalized, a plain set on a frozenset field. The layout guard exists to catch version skew, but the likeliest skew is same-name/flipped-meaning rather than a changed field list -- particles_ambiguous inverted sense between v1's never-given set and v2's may-be-given set without changing its name. A 2.0-alpha pickle carrying the old sense passed the name check and loaded silently inverted. Call __post_init__ after restoring state. That re-runs normalization and all four invariant checks, and rebuilds _cap_map for free -- the hand-rolled rebuild it replaces was already duplicating that line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
020e5ad fixed .get() being swallowed by _RegexesProxy.__getattr__ but stopped at the one method that had a deprecation promise attached. items(), values() and len() had the identical failure -- v1's regexes was a dict subclass, this proxy is not, and the catch-all __getattr__ claims every unimplemented mapping name as a regex lookup: CONSTANTS.regexes.items() -> AttributeError: no regex named 'items' That is the exact confusing message the get() fix was written to eliminate, and items()/values() are the next most likely v1 idioms after get(). Spell out the read half of the mapping surface, and say in the comment why a proxy has to. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
182249a's comment claimed the particles_ambiguous union "reproduces" v1 for a config that puts a word in both bound_first_names and non_first_name_prefixes. It reproduces two of three shapes. v1's join has a reserve_last guard, so with only two pieces it does not fire: "dos Santos" 1.4.0: last='dos Santos' here: given='dos' v1's rule is piece-count dependent; a static vocabulary set cannot express that, so the deviation is irreducible without reintroducing the raise on a config v1 accepted. Narrow the claim to what the code does and pin the divergent case as a deviation, so it is recorded rather than discovered later as a bug. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Each verified against a live 1.4.0. - release_log: assigning empty_attribute_default raises AttributeError, not TypeError. - release_log + migrate: only the mapping manager's AttributeError lists the known keys; the regexes proxy says "no regex named 'typo'". The two pages had also disagreed with each other. - migrate: v1's CONSTANTS.regexes.typo returned EMPTY_REGEX, not None. None was the capitalization_exceptions behavior. - README + migrate: "four removals ... which all raise on contact" was false for one of the four. A subclass overriding a parsing hook gets a DeprecationWarning and parsing proceeds -- and DeprecationWarning is suppressed by default outside __main__, so it is the opposite of raising on contact. This matters: it is the one pre-upgrade check item a reader could reasonably skip. - release_log: the "Smith, Dr." example claimed a title was being routed to first in 1.x. It was not -- 1.4.0 already gave title='Dr.'. What moved is the pre-comma piece, first -> family. The bullet now describes the change that happened instead of one that didn't. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Test gaps:
- __setstate__ now rejects a bare str component list. A str is
iterable, so the per-entry rebuild split its CHARACTERS and produced
a plausible-looking wrong name ("John" -> first "J o h n") rather
than failing at the load site. Only reachable from foreign or
hand-built state, since v1 stored lists.
- Added a test that restores a FOREIGN state dict carrying a multi-word
suffix entry. The parametrized round-trips all go through v2's own
__getstate__, which is self-consistent by construction, so none of
them exercised the 1.4-produced-blob case the fix exists for. It
passes as written; it was simply untested.
- Added a test importing all seven guarded config modules. An
import-time assert only runs if its module is imported, and
maiden_markers is lazy -- its guard fired only incidentally, whenever
some other test happened to build a default Lexicon.
Docs and comments:
- locales.rst: a pack's Lexicon fragment is validated standalone,
before the union onto the base, so a fragment that marks a word must
also carry it. Latent today (both shipped packs are policy-only) but
a trap for the next pack author.
- titles.py: the normalization comment was past tense about a bug fixed
in the same commit, so a later reader would find the claim false and
not know if it was stale. Restated as the hypothetical, which reads
correctly forever.
- _config_shim.py: name the drift-guard test instead of describing it.
Not changed: the "spec §2" citations flagged as dangling (the specs
directory is gitignored). They predate this branch and appear four
times in _facade.py; fixing one would be inconsistent, and fixing all
is its own change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A second review of the remediation found the per-word rule was wrong
too, in the opposite direction from the whole-entry rule it replaced.
Both are now gone.
The lookup key is the space-joined run of Role.TITLE tokens, built by
the PARSE rather than drawn from this vocabulary. It can contain a
multi-word TITLES member ("chargé d'affaires" is one, and the per-word
rule rejected it), and it can contain a conjunction, because 'and' in
"Sir and Dame Alex" is tagged Role.TITLE. No static relation over these
sets decides reachability. Two attempts each rejected working configs,
while the condition they guarded -- an entry nothing consults -- is
inert. Guarding a harmless case cost three broken configurations, so
the guard goes.
The shim now TRANSLATES rather than filters. v1 joins the raw title run
and applies lc(), which strips only the whole string's edge periods, so
an interior word keeps its own ("lt. col"); v2 normalizes per token and
then joins ("lt col"). Re-folding per word converts between them.
Filtering had silently swapped given and family for every multi-word
honorific containing an abbreviation or a conjunction. Verified against
a live 1.4.0: "Grand Duke John", "Lt. Col. Smith" and "Sir and Dame
Alex" now all match.
Also fixed, all from the same review:
- _normalize now strips to a fixed point. strip().strip(".") left
periods-around-whitespace half done ('. a .' -> ' a ' -> 'a'), so any
value re-normalized later changed under its owner -- which the
unpickle revalidation had just made reachable.
- __setstate__ RAISES on unnormalized state instead of quietly
correcting it. Silently rewriting caller data on load would have made
it a fourth undocumented transformation.
- The suffix_acronyms_ambiguous/suffix_words invariant kept, but the
shim now subtracts, so a v1-legal config translates instead of
raising at parse time naming a field (suffix_words) that does not
exist on v1's Constants.
- The facade's setstate guard covers Mapping and bytes and checks entry
types, not just bare str. A dict silently loaded its keys.
- _RegexesProxy gained copy(), and reports dict mutators as unsupported
rather than as missing regexes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The translation added in 23ee08e handled the reachable cases correctly but widened the degenerate ones, in both directions. v1's lookup key is lc(" ".join(title_list)) -- always single-spaced, never empty. So: - An entry holding a whitespace run ("grand duke") could never match there. Translating it collapsed the run and started matching, turning an inert entry into a behavior change. - An entry that is all periods (".", "..") folds to "" under per-word normalization, which _normset rejects -- so a config 1.4.0 accepts and ignores raised ValueError at parse time, naming a field that does not exist on v1's Constants. The same complaint 23ee08e leveled at the suffix_words error, reintroduced two lines away. Filter to entries v1's key shape can produce, then drop any that fold away. Both verified against a live 1.4.0. Two comment corrections from the same review: - the suffix_acronyms_ambiguous subtraction claimed parity with v1; it trades a raise for the two-piece deviation already documented below it ("Jack Ma" is last='Ma' in v1, suffix='Ma' here), so say that rather than claiming equivalence. - __setstate__ said "not written by nameparser" about state that an earlier commit on this branch could legitimately have written, before _normalize converged. Now "not written by this version". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The subtraction in 23ee08e traded a raise for something worse. It dropped the word from suffix_acronyms_ambiguous, which ungates it -- and v1 does NOT ungate. v1's is_suffix already accepts an ambiguous acronym through the acronym branch, and reserve_last keeps it as the surname, so adding it to suffix_not_acronyms is INERT there: c.suffix_not_acronyms.add("ma"); "Jack Ma" 1.4.0: first='Jack' last='Ma' (same as unconfigured) 23ee08e: first='Jack' last='' suffix='Ma' A person with no family name, silently -- from a change whose stated purpose was avoiding a loud error. Subtract from suffix_words instead, which reproduces v1's inertness. The test that was supposed to cover this used "John Smith jd", where v1 yields suffix='jd' with or without the config, so it passed on a case the translation does not decide. Rewritten to "Jack Ma", where it does. Also from the same review: - The justification for removing the given_name_titles guard cited "chargé d'affaires" as a multi-word TITLES member that would be rejected. Neither version ever tags it -- v2 builds its key per token, so a space-bearing vocabulary entry cannot appear in one. The conjunction case ("sir and dame", where 'and' is tagged Role.TITLE) is real and sufficient on its own; the false premise is struck from the comment and from the test that pinned it. - A stale comment still pointed at the per-word check deleted in the same commit. - The unpickle drift check covered ten fields and silently re-canonicalized the eleventh; capitalization_exceptions now raises like the rest. Every configuration broken across this session's four attempts now matches a live 1.4.0: grand duke, lt. col, sir and dame, ma-as-word, all-period entry, and E.S.Q. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Four rounds of review found the same class of mistake repeatedly: inferring an invariant from data that looked inconsistent, then editing the data or config to fit. Two of the notes below are anti-cleanup warnings for exactly the fences that got removed. Gotchas: - Comparing against v1 needs PYTHONSAFEPATH=1 and a directory outside the worktree, or --isolated still imports the branch as "v1" and every comparison reports parity. This produces false confidence rather than an error, which is the worst failure mode a verification method can have. - 'esq' is in both suffix sets on purpose; the branches normalize differently, so it is coverage of two spellings, not duplication. Deleting it lost the family name for "John Smith E.S.Q.". - Lexicon.given_name_titles is deliberately unvalidated, with the reason: a conjunction inside a title run is itself tagged Role.TITLE, so no static relation over the vocabulary decides reachability. Two guards, three broken configurations. - _normalize must reach a fixed point, now that storage and match-time share one fold and unpickling re-validates. 2.0 conventions: - Invariants guard harm, not no-ops. The criterion that would have prevented most of the above: construct the config it forbids and check what actually breaks. - The shim translates; it never raises on a config v1 accepted and never silently changes the parse. Documents all four transformations with their v1-reachability arguments, and the testing rule that would have caught the last bug -- test the case the translation DECIDES, not one where both branches agree. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
v1 reads "John Smith MA" as suffix='MA' and "Jack MA" as last='MA'.
That is not inconsistency — it is evidence-weighing. With only two
pieces, "one of them is a credential" is the less likely reading, so
the ambiguous acronym stays the surname; with three, peeling it still
leaves a given and a family name, so the credential reading wins.
2.0 had generalized the period gate over every position, making a bare
ambiguous acronym never a suffix, so "John Smith MA" came out
family='MA', middle='Smith'. That was an unclassified divergence, and
the wrong call: MA after a full name really is more likely a degree.
Root cause is the same shape as the rest of this session's bugs. In v1
suffix_acronyms_ambiguous has exactly ONE use site -- parse_nicknames'
delimited-content escape, deciding whether "(JD)" is a nickname or a
suffix. It is a delimiter-path disambiguator, not a vocabulary
property, and 2.0 promoted it to a universal rule.
Narrow it to what it can decide: peel a bare ambiguous acronym only
when at least two name pieces remain. This is v1's reserve_last
restricted to the ambiguous set -- 2.0 still peels UNambiguous suffixes
when nothing is left ("Smith PhD" -> suffix, a classified fix),
because there the vocabulary is not in doubt.
Verified against a live 1.4.0: 9 of 10 shapes now identical, the tenth
being that classified PhD fix. Four case-table rows pin both halves of
the rule, and the release-log bullet now describes it instead of
claiming periods are the only path.
Case rows use uppercase "MA" where a suffix is expected, so the
intended reading is legible without running the parser. (Case itself is
not yet an input to the decision; it could reasonably become one.)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
particles_ambiguous emitted PARTICLE_OR_GIVEN; suffix_acronyms_ambiguous
emitted nothing, for the structurally identical situation -- a word in
an ambiguous vocabulary where the parser picks one of two readings.
"parse('derek gulbranson MA').ambiguities" was empty even though the
parser had just chosen credential over surname.
Two kinds now emit:
SUFFIX_OR_FAMILY (new) at the peel in _assign, in BOTH directions --
"John Smith MA" reports that MA was read as a suffix, "Jack MA" that it
was read as the family name. Both are guesses.
SUFFIX_OR_NICKNAME (reserved since the core API landed) at classify,
for delimited content the vocabulary cannot settle: "(MBA)" escapes to
suffix on vocabulary alone, so "(JD)" keeping the nickname reading was
a silent coin-flip. Emitted at classify rather than at the escape in
extract, which runs before tokenize and so has no token index to
point at.
The emission has to be at the DECISION site, not on tag presence. In
"Joao da Silva do Amaral de Souza" the token 'do' carries
vocab:suffix-ambiguous but sits mid-name and is never at the peel
boundary -- no choice was made, so nothing is reported. Same reasoning
excludes "Ma, Jack", where a comma fixes the family before the question
arises: that mirrors the existing rule that PARTICLE_OR_GIVEN is not
emitted on the FAMILY_COMMA path. An ambiguity is a property of a
decision, not of a token.
Corpus impact is ~1%: 5 of 486 names.
test_every_ambiguity_kind_has_a_registered_trigger caught the new kind
immediately and the case table's exact-ambiguities assertion caught all
six affected rows -- both guards did their job.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The constraint that shaped the two new emitters is not obvious from either side, and ORDER is still reserved -- whoever wires it next needs the rule, and users need to know why a name containing an ambiguous word can report nothing. Three places, each for its own audience: - concepts.rst, in the Honest ambiguity essay: an empty ambiguities means the parse faced no fork, not that every word was unambiguous. Uses the two cases that surprised me -- 'do' mid-name in "Joao da Silva do Amaral de Souza", where nothing chooses, and "Ma, Jack", where the comma settles it before the question arises. - AmbiguityKind's docstring, which is what modules.rst renders into the API reference: the same point in two sentences, for a reader who never opens the concepts page. - AGENTS.md: the implementer rule. Emit at the decision site, never by scanning for a vocab:*-ambiguous tag; report both directions of a two-way fork; register a trigger in _AMBIGUITY_TRIGGERS and pin the kinds in the case table. Notes that the decision site already holds the token index and detail text a tag scan would have to rebuild. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
AmbiguityKind.UNBALANCED_DELIMITER has always documented itself as "a
delimiter opened without closing (or closed without opening)", but only
the first half existed. The scan is opener-driven -- it finds an open
and looks rightward for its close -- so a close with nothing to its
left was never in the search space:
'Jon "Nick Smith' -> unbalanced-delimiter
'John Smith)' -> (nothing)
No design reason, just the direction of the walk. Both signal the same
malformed input.
Sweep for boundary-valid closes that no match consumed, reusing
_close_ok -- which is what keeps apostrophes out of it. Verified on the
486-name corpus: "O'connor", "O'B.", "Queen's" all have close_ok=False
because the apostrophe is mid-word, so none of them fire.
Offsets already reported as unmatched OPENS are skipped, so a symmetric
delimiter that satisfies both boundary tests still yields exactly one
ambiguity ('John " Smith').
One corpus name newly reports: "Mari' Aube'", two word-final
apostrophes. That is arguably correct -- a trailing apostrophe really
is ambiguous between punctuation and a closing quote -- and the
ambiguity is advisory, so the parse is unchanged (given "Mari'", family
"Aube'"). Worth knowing it will be commoner on transliterated
Arabic/Hebrew/Slavic data, where word-final apostrophes are ordinary.
Found by auditing the existing emitters against the decision-site rule
documented in the previous commit.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The close sweep added in d993db0 flagged "Mari' Aube'". It should not: an ambiguity means the part could REASONABLY read two ways where it sits, and a "'" after a word character cannot -- it is an apostrophe. "'" is the only delimiter character that occurs inside and at the end of real name parts, and the least likely of them to mark a nickname. The same position with a quote IS ambiguous, because quotes do not appear inside names, so the carve-out is deliberately this one character: "Mari' Aube'" -> 0 ambiguities 'Mari" Aube"' -> 2 ambiguities #273 excluded the curly apostrophe from the delimiter set outright for this reason. The straight one has to stay a delimiter (v1's quoted_word), so it gets the narrower treatment here. The carve-out is about what PRECEDES the character, so an apostrophe opening a quote is untouched: "John 'Jack Smith" still reports an unmatched open. Also allows a preceding period, for initials ("O'B. Smith'"). Corpus back to 0 of 486 reporting, where the naive close sweep had 1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Last gap from the emitter audit. The kind carried no tokens, so the
ambiguity was locatable only by parsing an offset back out of its
detail string -- which is prose, not an API. The stray character does
survive into a token ('"Nick', 'Smith)'); nothing was pointing at it.
extract_delimited runs before tokens exist, so PendingAmbiguity gains
an `origin` character offset that tokenize resolves to the containing
token's index once the stream is built. Stages after tokenize set
indices directly and leave origin None.
Every emitted kind now carries its tokens:
'Jon "Nick Smith' unbalanced-delimiter ['"Nick']
'John Smith)' unbalanced-delimiter ['Smith)']
'Van Johnson' particle-or-given ['Van']
'John Smith MA' suffix-or-family ['MA']
'JEFFREY (JD) BRICKEN' suffix-or-nickname ['JD']
'Smith, John, Extra, Jr.' comma-structure ['Extra']
The docstring's "May carry no tokens" is narrowed to what remains true:
a character inside a masked region belongs to no token.
test_stage_field_ownership caught tokenize writing a field it did not
own before -- fixed, and classify's entry corrected in the same pass,
since its SUFFIX_OR_NICKNAME emitter had the same omission and only
passed because no case-table row triggers it yet.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
All three are mine, from the previous round, found by review.
1. Only one SUFFIX_OR_FAMILY survived per segment. ambiguous_pick was a
single slot the peel loop overwrote, so "John Smith MA JD" made two
coin-flips and reported one -- defeating the point of reporting.
Collect them instead. The sibling SUFFIX_OR_NICKNAME emitter already
did this correctly, so it was an oversight, not a design.
2. The detail string was wrong under two of three name orders. The
comment claimed the unpeeled piece "stays the last name piece -- the
family name under every order"; _name_positions maps a two-piece
name to [FAMILY, GIVEN] under FAMILY_FIRST, so it is the GIVEN name
there and the text said otherwise. Ambiguity.detail is public, so
this misinformed callers. Now reads the role back after assignment.
3. Origin resolution was quadratic, and so was the check it fed.
Resolving each ambiguity's offset rescanned every token, and
ParsedName.__post_init__'s subset check then did `tok not in
self.tokens` per referenced token -- O(ambiguities x tokens) twice
over. Reachable because the new closer sweep can emit one ambiguity
per delimiter character. Bisect over the (already sorted) token
starts, and hash the token tuple once:
') ' * N before after
N=1600 182 ms 12 ms
N=3200 -- 25 ms (linear)
The subset check was pre-existing but only became reachable at scale
once ambiguities started carrying tokens.
Verified: differential harness 486 names, 2 intentional diffs, 0
unexplained. The review separately confirmed the 8147ac6 peel change
against 304 generated shapes under all three name orders -- 86 changed,
all 86 from disagreeing with v1 to matching it, 0 regressions.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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>
Third instance of the same mistake in this emitter, and the same root
cause both previous times: keyed on a SHAPE rather than the DECISION.
When the piece after an ambiguous particle is a suffix, the chain's
inner scan never advances, so merge(k, k+1) folds a piece into itself.
Nothing is chained and the particle stays a lone leading piece -- but
the record was appended before the merge, keyed only on "is a prefix"
plus the ambiguous tag. Result:
'Dr. Van Jr.' -> given='Van' AMB "'Van' was chained into the
family name"
The detail asserted the opposite of what happened, for all 39
particles_ambiguous members. For do/freiherr/st the token lands in the
TITLE instead.
Worse, the no-op leaves exactly the shape _assign triggers on, so 36 of
39 double-reported the same token from two stages.
`j > k + 1` is the discriminator -- it distinguishes a chain that
claimed a following piece from one that claimed nothing -- and it fixes
both defects, since the no-op was the only overlap between the two
emitters' domains. They now partition cleanly.
Also fixed the detail's wording. _group runs before assignment, so it
cannot know which field the chained piece lands in: "Dr. Van Johnson de
la Cruz" puts it in GIVEN, not family. This is the same defect the
role-readback cured in _assign one commit earlier -- the lesson did not
carry to the new emitter, and _group cannot read a role back, so the
text now describes the decision rather than guessing an outcome.
A "Dr. Van Jr." case row would have caught both criticals; 1539 tests
passed with them present because the only titled-particle test used an
UNAMBIGUOUS particle, pinning vocabulary instead of the decision.
Verified: 0 spurious reports across all 39 ambiguous particles; corpus
486 names, 2 intentional diffs, 0 unexplained.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two findings from the review.
The kind has one name and two causes, and they were sharing one
template. The acronym branch turns on whether periods are written; the
numeral branch turns on the letter being a numeral at all. Sharing the
first produced:
'V' written without periods is both a post-nominal and an ordinary
name; read as a suffix rather than a name part
"written without periods" is the MA/M.A. distinction, which does not
exist for V, and the flattened "a name part" hid that the declined
reading is specifically a middle initial. The kind's own docstring says
detail names what was declined; it could not even tell you which branch
fired. Each branch now writes its own.
And the documented claim is narrowed, for the second time. The review
found a THIRD silent site for the roman-numeral fork -- the
SUFFIX_COMMA tail, decided in _segment by is_suffix_lenient and stamped
in _assign's blanket tail loop, so it is neither of the two comma
exceptions already documented. "John Smith, V" reports nothing.
Rather than chase coverage a third time, the claim no longer depends on
it. It said an empty tuple means "none of the forks listed here came
up", which is only true if every listed kind is emitted everywhere its
fork occurs -- a property I cannot verify and have now been wrong about
twice. It now says reporting is deliberately partial, names the comma
paths as the known quiet ones, and states the only thing that holds
regardless: a non-empty tuple is a signal, an empty one is not a
guarantee.
Verified: corpus 486 names, 2 intentional diffs, 0 unexplained.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The ambiguity convention added earlier covered where to emit and how to test presence. Six review rounds and Derek's scope call exposed four gaps, three of them things I got wrong AFTER writing the rule. - A kind is worth adding only if a reader would hesitate too. The existing rule said where to emit, never whether a fork deserves a kind at all. "Smith, John V" reads as a middle initial to anyone, so reporting it is noise that teaches callers to ignore the field -- reachability of the second branch is necessary, not sufficient. This is what stopped the feature growing indefinitely; every reviewer kept finding more reachable forks, and reachability was the wrong measure. - A branch that runs but changes nothing is not a decision. merge(k, j) executes when j == k + 1, folding a piece into itself; keying on "the code got here" reported a fork for all 39 ambiguous particles on "Dr. Van Jr." and double-reported with _assign. Third instance of the same mistake, the last one after the rule was written down. - Pin the decision, not the vocabulary. The only titled-particle test used an UNAMBIGUOUS particle, so it walked the right code path and proved nothing about the branch under test -- two criticals passed 1539 tests. - Document the positive direction of a partial property. "A non-empty ambiguities is a signal" is checkable; "an empty one means no fork occurred" is a universal negative needing exhaustive verification. It was written twice and falsified twice, at sites I had not audited. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_overlaps scanned the whole masked list, and the unmatched-close sweep added last week calls it once per delimiter character found. On input with many matched pairs that is O(hits x pairs): 400 pairs spent 5.35ms here against 2.52ms before the sweep existed, growing 2.55x per doubling. masked is sorted and non-overlapping by construction -- the scan is position-driven, so every later match starts at or after the previous one's end -- which makes this the same bisect the origin resolution in _tokenize already uses, for the same reason. Scaling is linear again (1.9x per doubling, 800 pairs in 5.2ms). Two smaller wins in the same sweep: iterate distinct closes (the defaults list '”' twice, so the text was scanned 11 times for 10 closes), and reuse _unmatched() rather than restating its detail template inline. Also guard the origin resolution on there being an origin to resolve -- it was building starts and a closure on every parse for the overwhelmingly common no-ambiguity case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The lookup key was built in three places and one of them disagreed.
post_rules joins normalized title tokens ('lt col'); the v1 shim
translates first_name_titles the same way; but _normset stored native
2.0 entries with _normalize per whole entry, which leaves interior
periods alone. So Lexicon(given_name_titles={'lt. col'}) stored a key
the matcher can never build, and the entry silently matched nothing --
while the identical config through the Constants shim worked.
Give the rule one home: _title_key(words) in _lexicon, called by all
three. This is the field with no validation by design (two guards were
tried and both broke working configs), which makes a silent no-op its
most likely failure -- worth removing the one instance we control.
Strictly widening: 'lt col' folds to itself, so no config that works
today changes; entries with interior periods or repeated spaces start
working instead of being inert.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_group_segment widened its return to a 3-tuple to carry particle forks out to the caller, which then wrote the message ~110 lines from the branch that earned it, behind a suppression flag the caller had already computed. _assign_main solves the same problem by taking the ambiguity list as an out-parameter; do that here too. The message now sits with the comment explaining why the fork exists, and one of the two family-comma suppression comments goes away. Same treatment for _assign's roman-numeral fork: its wording depends on nothing assigned later, so it emits at its trigger rather than riding a (piece, cause) tag through to a post-loop branch. That leaves ambiguous_picks holding one cause, which is what made the tag necessary. Also: collapse the two adjacent bare_ambiguous tests into one (the shared "either way, record the fork" was invisible between them), drop the unreachable role fallback in favour of an assert that says the invariant out loud, and put the cheap selective tag test before the per-piece title() scan in the group guard. No parse output changes: 1549 tests, and the differential corpus holds at 486 names / 2 intentional diffs / 0 unexplained. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every vocabulary module carried the same all(w == w.strip().lower()) assert under a near-verbatim three-line comment, four of them differing only in the set name. One helper, assert_normalized(name, entries), states the rationale once and names the offending entries when it fires (the old copies only said that something was wrong). The fold stays the weaker w.strip().lower() rather than _lexicon's _normalize: entries like 'esq.' are legitimate data here, and config cannot import _lexicon anyway -- _lexicon imports these constants. The relationship asserts (FIRST_NAME_TITLES <= TITLES, the SUFFIX_ACRONYMS_AMBIGUOUS pair, the deliberate non-disjointness note) stay in the modules that own them; those encode facts about the data rather than hygiene. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_lexicon.__setstate__: compute the drift set once per field instead of three times, in _VOCAB_FIELDS declaration order rather than alphabetized by formatted string, and drop the intermediate dict that existed only to host a cast. _types.ParsedName.__post_init__: nest the token-subset check under the ambiguities guard rather than giving one local two types to skip work the loop already skips. _config_shim: inline a local used once. tests: drop a bare-string __setstate__ test fully subsumed by the parametrized case above it, and stop parsing the same input twice in the unmatched-close assertion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_group and _assign each report one branch of this fork and coordinate
only through _group's `j > k + 1` guard, which mirrors _assign's
reachability by hand. Nothing checked the mirror: reachability drift in
either stage would over- or under-report, and only a case pinning that
exact name would notice.
Generate the shapes rather than trusting a corpus. The boundary is a
suffix straight after the particle ("Dr. Van Jr."), where the chain is a
no-op and only _assign should speak -- a shape plausible-name corpora
have little reason to hold, and indeed the 486-name differential corpus
does not. Swept over every ambiguous particle, so a vocabulary addition
is covered without editing the test. Verified to fail in both
directions by loosening and tightening the guard.
Asserts the count, not which stage spoke, on purpose: whether a fork was
decided by a vocabulary merge or by position is NOT recoverable from the
finished parse. 'Dr. aan Johnson Jr.' and 'أبو بكر أحمد' end with the
same roles and the same tags, and only one is a fork -- so a
reconstruction would have to re-implement _group instead of checking it.
That is also why the two emitters stay where the decisions are made.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
parse(".,") returned given='.' and bool() True -- faithful v1 parity
(1.4 gives first='.' too), but it defeats the bool(parse(x)) idiom 2.0
documents as "did I get a name?". A caller filtering junk with
`if parse(x):` was fooled by all-punctuation input.
assemble now empties a parse whose surviving tokens carry no
alphanumeric character anywhere. isalnum() is Unicode-aware, so every
real name in any script has content -- the guard fires only on pure
punctuation/symbols ('.', '- -', the stray '∫≜⩕'). Junk embedded in a
name with real content ('John . Smith' keeps its dot) is left alone,
since that parse is already truthy and bool() is not misled.
Deliberate v1 divergence, both APIs (they share the core). Zero
differential impact: the corpus's three content-free names ('', '()',
',') already parsed empty for other reasons. Release-log note added.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The table was keyed by destination field, which forced conjunctions
into the `family` row as "conjunctions inside a surname" -- misleading,
since the very next doctest shows `and` joining two GIVEN names. Keying
by word type gives conjunctions one honest row ("both neighbors,
whichever field they join") and lets the field column read as a map:
the five field-specific attachers cover title/suffix/given/family/
maiden, and the two fields you can't get by attachment (middle,
nickname) are exactly the ones absent.
Example words for titles and suffixes are drawn from the shipped
constants (dr/sir/prof/capt, phd/md/jr/esq).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Each word type now links to its full set on the API reference (the
config-module sections render the whole {...}), so the few example
words are no longer needed inline -- they move to a restored Example
column showing a parse. Drops the Words/Field repetition the inline
lists caused.
Also tighten the conjunction row (Field "any"; "the words on both
sides") and spell out in the lead-in why titles/suffixes say "adjacent"
while the others say "the following word": the first attach only to
same-kind neighbors, the rest reach forward to the next word of any
kind.
Co-Authored-By: Claude Opus 4.8 <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
3 participants
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.
Implementation branch for the 2.0 design — see the RFC in #285 and the umbrella issue #284 for the design discussion and feedback questions. Implementation learnings that change the design will land as amendment commits on #285, per the plan described there.
Progress
Span,Role,Token,Ambiguity/AmbiguityKind,ParsedName;Lexicon,Policy/PolicyPatch/apply_patch,Locale. Frozen, slotted, hashable, picklable dataclasses with eager fail-loud validation; ~110 dedicated tests undertests/v2/.render()/initials()/capitalized()/__str__)Parser/parse()/parser_for()/matches()+ shared case tableHumanNamefacade +CONSTANTSshim (existing test corpus as regression harness) — full v1 suite (1,240+ tests) reconciled and passing against the facade; differential harness (tools/differential/) verifies 1.4-on-PyPI vs the facade over 486 corpus names with one classified diffru,tr_az) — shipped with the non-interference gate, Provide constants in non-Latin scripts (Cyrillic, Greek, Arabic, Hebrew) #269 default vocabulary, and CLI--localeNotes on what's here so far
TypeError, well-typed bad value →ValueError, enum lookups stayValueErrorper stdlib precedent.tests/v2/test_layering.py), and the seven-field canonical order derives fromRoledeclaration order everywhere — no ordering is ever stated twice.nameparser/configdata modules through 2.x, including the newmaiden_markers.py(Recognize maiden-name markers like "née" and "geb." (Jane Smith née Jones) #274 — marker words like "née"/"geb."; decisions recorded on the issue).🤖 Generated with Claude Code