nameparser 2.0 implementation#288
Draft
derek73 wants to merge 284 commits into
Draft
Conversation
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>
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #288 +/- ##
==========================================
+ Coverage 97.67% 98.08% +0.40%
==========================================
Files 13 37 +24
Lines 947 2350 +1403
==========================================
+ Hits 925 2305 +1380
- Misses 22 45 +23 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Every matrix job was silently testing CPython 3.11: uv sync honors .python-version (pinned 3.11 by the floor bump) over setup-python's interpreter, so 'build (3.14)' ran the suite on 3.11.15 (verified in the PR #288 run logs). UV_PYTHON makes the matrix real. With that fixed, add 3.15 (final 2026-10-01) as a non-blocking pre-release job so interpreter breakage surfaces while 2.0 is being built; flip it to blocking when 3.15 goes final. Locally green today on 3.15.0b3: pytest, mypy, ruff. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <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>
Two quadratics shipped this cycle (_extract's mask-overlap scan, ParsedName's token-subset check) and both were caught in review rather than by a test. The existing perf tests cannot catch that class: they bound absolute cost on constant-size input, and their workload is a short name with no delimiters, so a stage quadratic in the number of delimiter pairs stays far inside the one-second bound. Measured: that workload runs in 0.067s against a 1.0s limit. Add a growth test instead -- time a repeated pathological unit at n and 4n, assert the ratio stays near the linear 4. Seven units, one per stage inner loop (matched pairs, the open==close path, both unmatched sweeps, long token streams, comma segments, title chains). Calibrated against the real quadratic rather than guessed, which mattered: the growth is mixed, so the textbook 16x never appears at a testable size. At n=200 the quadratic measures 7.7x and an 8.0 bound passes it -- the first version of this test was vacuous. At n=400 the populations separate (clean 4.1-4.3, quadratic 9.2), so the bound sits at 6.5. Verified both ways: passes clean, and reverting the _extract fix fails exactly the two shapes that build masked spans. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The existing corpus comes from v1's own test suite, which makes it
structurally blind to everything 2.0 added: v1's authors had no reason
to write a test for a typographic nickname delimiter or a Cyrillic
title, so no rule was ever needed to classify those behaviors.
Harvest the adversarial half instead -- names users REPORTED as broken,
taken from HumanName("...") calls and quoted phrases in issue bodies.
166 of the 198 names were absent from the existing corpus, and the
first run paid for the exercise: five intended-but-unclassified 2.0
behaviors (#273 typographic delimiters, #269 non-Latin vocabulary) and
one shape no test had considered.
That shape is a LEADING "Ph. D.". v1 healed the split 'Ph.'/'D.' pair
only when it trailed; leading, it split them into title 'Ph.' and given
'D.' and pushed the real given name to middle. 2.0 keeps the credential
whole. Classified as a fix, pinned in cases.py, noted in the release
log. The toml's deliberate "leave trailing Ph.D. unclassified so a
regression fails" strategy is what surfaced it, and the new rule is
anchored to ^ so trailing stays unguarded.
Kept as a separate corpus with its own builder: corpus.jsonl is
reproducible forever from an immutable git ref, while an issue tracker
is mutable, so regenerating this one is an explicit, reviewable act.
compare.py now reads every corpus*.jsonl by default -- a corpus you
have to ask for by name is one that stops being run -- taking the
harness from 486 names to 652, 18 intentional diffs, 0 unexplained.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every existing property test varies the input text while pinning Lexicon.default() and Policy(). That leaves 2.0's largest new surface covered only by hand-written cases, which is backwards: the vocabulary and the switches are what a user is invited to change, so they are what is most likely to receive a combination nobody tried. Two properties, from opposite ends: - A valid config must still parse totally. Generates Lexicon/Policy combinations and asserts parse never raises, spans still index the original exactly (the anti-#100 invariant, now under configuration rather than under the default vocabulary), and rendering survives. - An invalid config must fail cleanly. Feeds plausible caller mistakes (bare string, mapping-for-set, bytes, None, entries normalizing to empty) and asserts only ValueError/TypeError escape -- anything else means the bad value got past validation and blew up downstream, where the message no longer names the field the caller typed. A config that IS accepted must then survive a real parse, since constructing successfully and dying one stage later is the same bug. Two details that decide whether this tests anything. The input is built from the lexicon's OWN words: fuzzing config against unrelated text is near-vacuous, because a random string essentially never contains a random vocabulary entry, so every configured set would sit unused. And the generator repairs a random draw into a legal Lexicon rather than generating one legally -- drawing dependent subsets directly makes the strategy tree deep and mostly rejected. Verified both can fail: a planted raise on a rare config combination (middle_as_family AND particles) is found by the first, and downgrading a documented TypeError to KeyError is found by the second. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_title_key was not idempotent. It joined per-word _normalize results
without dropping words that fold away, so an entry containing a
period-only word stored a key with a stray or doubled space:
'lt .' -> 'lt ' (was 'lt' before this branch)
'lt . col' -> 'lt col'
'. .' -> ' ' accepted; every other field raises
Three failures from one cause. The stored key can never be built at
match time, so the entry is silently inert -- the exact failure the
per-word fold was ADDED to remove. The empty-entry guard tests the
joined string, which is whitespace rather than empty, so this one field
lost that validation. And storage re-runs the fold on unpickle, so a
round trip raised "not written by this version of nameparser" about a
lexicon this version had just written.
It was also a regression, not merely a new edge case: 'lt .' stored
'lt' and worked before. My commit message claimed the change was
strictly widening; that was wrong. The constructor and add() also
disagreed, since dataclasses.replace re-runs __post_init__ and
converges on the second pass -- the signature of the bug.
The content-free guard dropped recorded ambiguities along with the
tokens, contradicting the comment eight lines below it. parse('(')
lost its unbalanced-delimiter report -- on exactly the input where
"was this malformed?" is the only question left to answer. Emptying
the name is still right; discarding the diagnostic was not.
compare.py's new default glob turned its loudest failure silent: no
corpus*.jsonl meant zero comparisons, "0 unexplained", and exit 0,
where the single hard-coded path it replaced raised FileNotFoundError.
Now it exits 1 on no files or an empty one, and prints per-file counts
so a corpus that shrinks is visible rather than merely absent.
Also from the review: narrow the #269 classification rule to Cyrillic
and to the three fields a title/conjunction shift can reach -- it had
put every Arabic name in the corpus, including ones that are currently
parity, into its shadow, and pre-excused Greek and Hebrew against which
nothing has ever been compared.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
test_properties.py hard-coded the two lists that drive the config fuzz. _SET_FIELDS was character-for-character _lexicon._VOCAB_FIELDS and _POLICY_FIELDS was dataclasses.fields(Policy), so an eleventh vocabulary field or a new Policy switch would have been silently un-fuzzed -- the invisible gap this layer exists to prevent. Derive both; test_policy.py already reflects over the dataclass this way. Two test modules had also each grown their own hard-coded corpus.jsonl loader, so both missed corpus_issues.jsonl entirely -- 198 reported names, 166 found nowhere else. That is precisely the case compare.py globs to avoid, and its comment says so, in the same commit that added the second file. One differential_corpus() helper in conftest, globbing corpus*.jsonl for the same reason. The particle-fork test goes from 486 names to 652, and the locale non-interference gate with it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Mutation testing of the seven shapes found one stage unreached: a
quadratic planted in group's prefix-chain loop went undetected by all
of them, because no shape repeated a particle. 'van ' catches it.
Also record what the guard deliberately does NOT cover. A conjunction
chain ('and ' * n) is already superlinear in shipped code -- 2.4x-2.9x
per doubling where every covered shape holds at 2.0 -- because group's
merge() re-flattens the whole accumulated piece on every call. Adding
that shape is the regression test for fixing it, so it is named in a
comment rather than quietly left out.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Lexicon._normset rejects both shapes by name, with dedicated messages,
because both iterate into something plausible but wrong. Policy had no
equivalent, so the same mistakes were accepted silently:
Policy(extra_suffix_delimiters={"dr": "Dr"}) -> frozenset({'dr'})
Policy(nickname_delimiters="") -> frozenset()
A mapping contributes only its keys; '' iterates to nothing at all, so
it stored an empty set rather than failing. Either way the caller reads
back a configuration they did not write -- the expensive failure this
library guards against everywhere else. Found by the config fuzzer,
which could not flag it itself: silent acceptance satisfies its "raises
only documented types" contract, so the test documented the asymmetry
instead of catching it.
One shared guard across all four collection fields, replacing the two
bespoke bare-string checks that covered patronymic_rules and
extra_suffix_delimiters but not the delimiter pairs. Wording is
deliberately parallel to Lexicon's.
Pre-2.0 strictness is free here: no released version accepted these,
since Policy is new in 2.0.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
merge() rebuilt the combined piece from scratch on every call:
pieces[lo:hi] = [[i for p in pieces[lo:hi] for i in p]]
A chain that merges into the same piece n times therefore copies
1+2+...+n elements, so the stage was quadratic in the length of the
chain. A conjunction run measured 2.4x-2.9x per doubling where every
other input shape holds at 2.0x; 32KB of "and " took 279ms.
Extend the first piece in place instead: 2.0x per doubling, and the
same 32KB input now takes 60ms. No piece list is aliased outside
_group_segment -- each starts life as a fresh [i], and the two callers
that hold one (the ph-d pair, the post-drop rebind) only read it before
the merge -- so mutating is safe.
Verified behaviour-identical rather than assumed: token texts, roles,
tags, spans, and ambiguity kinds/details/referents compared against the
pre-fix tree over 54,877 names (the 652-name differential corpus plus
every 2-, 3- and 4-way combination of conjunctions, particles, titles,
Ph./D. pairs, maiden markers and commas) -- 0 differences. Differential
harness unchanged at 18 intentional, 0 unexplained.
The scaling guard gains the "and " shape it had been documenting as a
known gap; it fails on the pre-fix code at 6.6x, so it is a real
regression test rather than a shape that happens to pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The previous commit added _reject_str_and_mapping to Policy but left
PolicyPatch carrying an inline copy of only the bare-string half, so
the mapping half never reached a patch -- all four union fields, not
just the one I noticed:
PolicyPatch(extra_suffix_delimiters={"dr": "Dr"}) -> frozenset({'dr'})
PolicyPatch(patronymic_rules={"turkic": "yes"}) -> {TURKIC}
PolicyPatch(nickname_delimiters={}) -> frozenset()
Worse than the Policy version it mirrors: the frozenset() coercion
destroys the evidence, so Policy cannot catch it later at apply time,
and a Locale pack ships exactly one of these. Call the shared helper
instead of re-implementing half of it -- re-implementing half of it is
how the two diverged in the first place.
name_order had the same unguarded mapping: a {Role: position} dict
iterates to the right three Roles in the right order. Harmless today,
since the result is validated against the three exported orders
regardless, but "iterating this yields something plausible but not
what you wrote" is one bug class and leaving one field out of it is
precisely what happened above.
The guard tests are now parametrized over BOTH classes, which is what
would have caught this: the pre-existing parity tests compare field
names and types, never validation behaviour. Added the positive case
too -- set, list, tuple, frozenset, generator and dict_items must all
still be accepted -- since nothing asserted the guards do not
over-reject. dict_items is the documented escape for an {open: close}
mapping and survives only because ItemsView is a Set, not a Mapping;
if that ever changes the test says so.
Message now names the way out rather than only the harm: {} (meant as
an empty set) and an {open: close} dict are the two mappings people
actually pass, and "contributes only its keys" helps with neither.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
the other new tests left unpinned The scaling guard certified a fix it could not reliably see fail. 6.5 was calibrated against the _extract quadratic (9.2 at base 400); the conjunction shape added later has a much weaker signal (6.3-6.6 at the same base), so the bound landed on the MEDIAN of the broken distribution. Measured against the pre-fix merge over 20 samples, it caught the regression 13/20 -- and a reviewer measured 9/20. My own "verified it fails" was one sample of a stochastic test that happened to land above the line. Raise the base to 800 rather than squeeze the bound into a gap that is not there: clean 3.95-4.34, the merge quadratic 7.93-8.06, so 6.0 has ~1.4x noise headroom and ~1.3x detection margin -- symmetric, where (400, 5.4) would have been 1.28x/1.16x. Verified 8/8 clean passes and 8/8 detections across repeated runs, not one. Costs 1.3s. The comment now records both bugs' signal strengths and says to re-derive the constant when a shape is added, which is the step that was skipped. Three tests that could not fail: - The Policy positive test asserted `cls(...) is not None`, a tautology for a dataclass. It passes for a guard that accepts a one-shot iterable, exhausts it, and stores frozenset() -- the exact "stored a value the caller never wrote" failure the guards exist to prevent. Assert the stored value, over all four fields and both classes: an over-strict guard on maiden_delimiters was invisible to all 2269 tests, because every other call site passes it a frozenset. - The content-free ambiguity KEEP was unpinned; reverting it left the suite green. Every junk row in the existing test is balanced or delimiter-free, so none of them exercises a born-empty report. - A bare pytest.raises(ValueError) now pins its message, and a parametrize case that collapsed to a duplicate under split() is replaced by one that does not. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two documentation defects from the aliasing review, which otherwise
cleared the merge rewrite: ~1.1M instrumented parses with no assertion
firing, and byte-identical output against the pre-fix spelling over
8,036 names x 16 configs (all three name orders, middle_as_family,
both patronymic rules, custom delimiters, both locale packs, the
HumanName facade and the Constants shim) plus 120,375 names x 8
policies of targeted chain fuzz.
merge() now REQUIRES lo < hi, where the old spelling did not. Every
call site passes it, but with lo >= hi the slice assignment would
insert rather than replace, putting a second reference to pieces[lo]
into the array so the next merge extends the same list twice. Stated
at the call boundary rather than guarded: a silent no-op return would
hide a caller bug, and the constraint only matters to someone adding
a call site.
AmbiguityKind.UNBALANCED_DELIMITER called an empty token tuple a "rare
exception" for a character inside a masked region. Since content-free
input started keeping its diagnostics there is a second source, and it
covers the canonical example -- parse("(") reports with tokens=()
because no token survived. The docstring now admits both.
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