Skip to content

Commit f1ef995

Browse files
authored
Merge pull request #441 from derek73/refactor/439-shared-piece-predicates
refactor(pipeline): piece-level predicates get their own module (#439)
2 parents fcebf79 + 7beecf1 commit f1ef995

9 files changed

Lines changed: 389 additions & 248 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ Add a dedicated `copy.deepcopy()` round-trip test for it too (see `test_regexes_
298298

299299
**Titles permanently shadow first names — be conservative** — any word in `TITLES` is always consumed as a title and can never be parsed as a first name. `"Dean"` is the canonical example: it's a common academic title *and* a common given name, so it is intentionally absent from the default titles (see `docs/customize.rst` — users who need it add it via opt-in `Constants`). Before adding a word to `TITLES`, ask: "Could this plausibly be someone's given name in any culture?" If yes, don't add it globally; it belongs in caller-supplied `Constants` instead. This same caution applies to international honorifics — `Prince`, `Sheikh`, `Frau` are all first names in some contexts. It also applies to any prefix sub-set gated on "never a first name": obscure-looking foreign particles are surprisingly often real given names — `Von` (Von Miller), `Vander` (Brazilian, also the Arcane character). When unsure, exclude — a missing member just means that name isn't auto-handled, whereas a wrong member misparses a real person.
300300

301-
**The period-abbreviation title inference runs at the head of the GIVEN-NAME part, not the head of the name** — an unrecognized multi-letter word ending in a single trailing period (`_assign._PERIOD_ABBREV`, a hand copy of the `period_abbreviation` regex, `{2,}` letters) is treated as a title in the leading title run, e.g. `"Insp. Jane Morse"` → `title='Insp.'`. "Leading" is per SEGMENT: `_peel_leading_titles` is called for NO_COMMA segment 0, SUFFIX_COMMA segment 0, and FAMILY_COMMA **segment 1**, so `"Morse, Det. Insp. Jane"` → `title='Det. Insp.'` and a lone `"Smith, Xyz."` → `title='Xyz.'` — long-standing, verified against 1.4.0, and the mechanism behind #296 (`"Smith, Jr."` → title, which the shape rule claims even once `jr` leaves `TITLES`). The docs said "leading word" until 2026-08-01 and were wrong for every comma path. It does not mutate `C.titles`, so the periodless form (`"Insp"`) is unaffected elsewhere. The `{2,}` length requirement — not a separate initials check — is what excludes single-letter initials like `"J."`; the same word after the given name is left as a middle name. **The inference OUTRANKS vocabulary where it runs**: `"Esq. Smith"` → `title='Esq.'` even though `esq` is suffix-only vocabulary, because the shape rule fires before anything consults the suffix sets. **And it runs in one direction only**: a trailing abbreviation has no structural counterpart and is matched against the suffix vocabulary alone, so a trailing TITLE word is not a title (`"John Smith Prof."` → `family='Prof.'`, and the comma path disagrees — `"Smith, Prof."` → `title='Prof.'`). Meanwhile `period_joined_vocab` resolves INTERIOR-period tokens (`Lt.Gov.`, `Msc.Ed.`) to title-or-suffix by vocabulary, and `_extract._suffix_shaped` treats any period-final delimited content as not-a-nickname. Four trailing-period behaviors, four different resolutions; unifying them is open design work, not settled. (#109; see `docs/usage.rst` "Titles you didn't configure")
301+
**The period-abbreviation title inference runs at the head of the GIVEN-NAME part, not the head of the name** — an unrecognized multi-letter word ending in a single trailing period (`_pieces._PERIOD_ABBREV`, a hand copy of the `period_abbreviation` regex, `{2,}` letters — it was assign's until #424 and group's until #439) is treated as a title in the leading title run, e.g. `"Insp. Jane Morse"` → `title='Insp.'`. "Leading" is per SEGMENT: `_peel_leading_titles` is called for NO_COMMA segment 0, SUFFIX_COMMA segment 0, and FAMILY_COMMA **segment 1**, so `"Morse, Det. Insp. Jane"` → `title='Det. Insp.'` and a lone `"Smith, Xyz."` → `title='Xyz.'` — long-standing, verified against 1.4.0, and the mechanism behind #296 (`"Smith, Jr."` → title, which the shape rule claims even once `jr` leaves `TITLES`). The docs said "leading word" until 2026-08-01 and were wrong for every comma path. It does not mutate `C.titles`, so the periodless form (`"Insp"`) is unaffected elsewhere. The `{2,}` length requirement — not a separate initials check — is what excludes single-letter initials like `"J."`; the same word after the given name is left as a middle name. **The inference OUTRANKS vocabulary where it runs**: `"Esq. Smith"` → `title='Esq.'` even though `esq` is suffix-only vocabulary, because the shape rule fires before anything consults the suffix sets. **And it runs in one direction only**: a trailing abbreviation has no structural counterpart and is matched against the suffix vocabulary alone, so a trailing TITLE word is not a title (`"John Smith Prof."` → `family='Prof.'`, and the comma path disagrees — `"Smith, Prof."` → `title='Prof.'`). Meanwhile `period_joined_vocab` resolves INTERIOR-period tokens (`Lt.Gov.`, `Msc.Ed.`) to title-or-suffix by vocabulary, and `_extract._suffix_shaped` treats any period-final delimited content as not-a-nickname. Four trailing-period behaviors, four different resolutions; unifying them is open design work, not settled. (#109; see `docs/usage.rst` "Titles you didn't configure")
302302

303303
**Cyrillic suffix regexes need `re.I` even when the pattern is suffix-only** — a Latin title-cased word (`Ivanovich`) keeps its suffix lowercase, so `re.I` seemed skippable; but an irregular Cyrillic suffix can be nearly the whole word (`ильич`), so title-casing capitalizes into the suffix itself (`Ильич`). `east_slavic_patronymic_cyrillic` shipped without `re.I` on the Latin reasoning and silently failed on capitalized irregular forms — don't assume Latin's title-case safety transfers to Cyrillic. (#185)
304304

docs/design/mechanisms.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,9 @@ Problem shape. A fact known during tokenization matters to a much later stage. C
5353

5454
Problem shape. "Which stage does X?" — asked before attributing behavior in prose, comments, or fixes. Contract statement. Each stage's docstring header declares what it consumes, produces and reads, and ParseState's docstring holds the cross-stage map, pinned by tests/v2/pipeline/test_state.py. How it works. A claim about which stage or layer does something is CHECKABLE — `parse(s).tokens` prints every token's role and tags — so check it before writing it; one plausible attribution sentence once shipped six times wrong (AGENTS.md's stage-attribution note). Lives in. nameparser/_pipeline/_state.py and every stage header. Reach for it when. Writing any sentence of the form "X happens before Y sees it."
5555

56-
## ONE-PREDICATE-PER-QUESTION — the stage that does not decide calls the one that does
56+
## ONE-PREDICATE-PER-QUESTION — one predicate answers it, and every other site calls that
5757

58-
Problem shape. Two stages need the same answer about the same input, and the one that does not own the decision is about to test for it. Contract statement. Where two sites ask the same question, exactly one predicate answers it, and the site that does not own the decision calls the deciding stage's own predicate — never a condition written to match it. How it works. A hand-written mirror agrees with its original only until one of them moves, and the drift is invisible in both directions: each site keeps passing its own tests while they disagree about an input neither covers. Five instances, every one found as a defect before it was found as a pattern — #319 lifted the wholly-suffix predicate into the vocabulary layer "so the comma decision and the honorific peel's segment test cannot drift apart"; #401/#421 lifted the trailing-numeral fork out of assign so the bound-given reserve stopped carrying a copy, its hand-written mirror having been falsified in review more than once — the lesson recorded there being that what must be mirrored is assign's WALK, not merely its condition; #425 replaced that reserve's hand re-derivation of the trailing peel with one function over the view the join would leave; #424 moved assign's leading-title test down because group's own `title()` does not see H2's unlisted abbreviations, so `Xyz. van Johnson` chained where `Dr. van Johnson` did not; #429 moved the no-name-segment test down because group asked by segment INDEX where assign asks by CONTENT. The destination follows the LAYER, not the topic: a predicate over token text goes to `_vocab`, one over pieces and tags goes to `_group` — not because grouping owns it, but because `_assign` imports `_group` and cannot be imported back. That import direction is this mechanism's limit, and it forecloses the alternative: where the reader comes AFTER the decider, record the answer on the state instead — `ParseState.order` is that shape, "Recorded rather than recomputed downstream, because the two can differ" — which is unavailable whenever the EARLIER stage is the one asking. The cost is a second evaluation of the same predicate, measured for #429 at 1.2–2.2% of a family-comma parse and 0% of every other; recording that number was the right answer there over plumbing a state field the two sites would not otherwise share. Lives in. nameparser/_pipeline/_vocab.py (is_wholly_suffix, is_trailing_numeral_suffix) and nameparser/_pipeline/_group.py (_is_suffix_piece, _is_leading_title, _leading_titles, _peel_walk, _peel_trailing, _segment_holds_no_name), each called by a stage that does not define it. Reach for it when. You are about to write a condition that mirrors, matches or "does what X does" — or you find a comment saying one does. Grep for the other site's predicate and call it instead.
58+
Problem shape. Two stages need the same answer about the same input, and the one that does not own the decision is about to test for it. Contract statement. Where two sites ask the same question, exactly one predicate answers it and every other site calls that one — never a condition written to match it. The predicate belongs to the QUESTION, not to whichever stage decides: it may sit in a leaf both stages import, and for the leading-title test it must, since the deciding stage is assign and group cannot import assign. How it works. A hand-written mirror agrees with its original only until one of them moves, and the drift is invisible in both directions: each site keeps passing its own tests while they disagree about an input neither covers. Five instances, every one found as a defect before it was found as a pattern — #319 lifted the wholly-suffix predicate into the vocabulary layer "so the comma decision and the honorific peel's segment test cannot drift apart"; #401/#421 lifted the trailing-numeral fork out of assign so the bound-given reserve stopped carrying a copy, its hand-written mirror having been falsified in review more than once — the lesson recorded there being that what must be mirrored is assign's WALK, not merely its condition; #425 replaced that reserve's hand re-derivation of the trailing peel with one function over the view the join would leave; #424 moved assign's leading-title test down because group's own `title()` does not see H2's unlisted abbreviations, so `Xyz. van Johnson` chained where `Dr. van Johnson` did not; #429 moved the no-name-segment test down because group asked by segment INDEX where assign asks by CONTENT. The destination follows the LAYER, not the topic: a predicate over token text goes to `_vocab`, one over pieces and tags to `_pieces`. Both are leaves the stages sit on. The piece layer got its own module only in #439 — until then those predicates collected in `_group`, not because grouping owned them but because `_assign` imports `_group` and cannot be imported back, so group was the one place both stages could reach; five had accumulated across four PRs before the module existed. Stage order is this mechanism's limit, and it forecloses the alternative: where the reader comes AFTER the decider, record the answer on the state instead — `ParseState.order` is that shape, "Recorded rather than recomputed downstream, because the two can differ" — which is unavailable whenever the EARLIER stage is the one asking. (The concrete assign→group import that forced the `_group` collection is gone since #439; what remains is the ordering it was a symptom of, and tests/v2/test_layering.py is where the leaf's contract is now written down.) The cost is a second evaluation of the same predicate, measured for #429 at 1.2–2.2% of a family-comma parse and 0% of every other; recording that number was the right answer there over plumbing a state field the two sites would not otherwise share. Lives in. nameparser/_pipeline/_vocab.py over text (is_wholly_suffix, and is_trailing_numeral_suffix — the #401/#421 instance, whose only caller since #439 is the shared peel rather than a stage) and nameparser/_pipeline/_pieces.py over pieces: _is_suffix_piece, _is_leading_title, _leading_titles, _peel_walk, _peel_trailing and _segment_holds_no_name are called by both stages, _is_title_piece and _trailing_start by group alone — `_trailing_start` being the one to know, since it answers where the trailing run begins and is what P2's chain and M2's walk stop at. tests/v2/test_layering.py holds each module's contract, and a piece predicate growing a dependency on a STAGE shows up there as a widened entry. Reach for it when. You are about to write a condition that mirrors, matches or "does what X does" — or you find a comment saying one does. Grep for the other site's predicate and call it instead.
5959

6060
## CLAUSE-CONTENT-OVERRULES-DELIMITER — content wins
6161

docs/design/rules.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ H2. Rationale: before a name, an abbreviation is almost always a
7676
not open: the vocabulary decides, and "Esq." is the postnominal
7777
it is.
7878
"Smith, Esq." → suffix="Esq."
79-
history: decisions.md#H2 · interacts: C1, P4 · implemented: nameparser/_pipeline/_assign.py, nameparser/_pipeline/_group.py
79+
history: decisions.md#H2 · interacts: C1, P4 · implemented: nameparser/_pipeline/_assign.py, nameparser/_pipeline/_pieces.py
8080

8181
H3. Rationale: compound titles are written as a run of title words,
8282
connectives included; a title word standing inside the name is
@@ -91,7 +91,7 @@ H3. Rationale: compound titles are written as a run of title words,
9191
Accepted: before a family comma the pre-comma text is wholly the
9292
family name (C1), title words included.
9393
"Dr. Smith, John" → family="Dr. Smith"
94-
interacts: C1 · implemented: nameparser/_pipeline/_group.py
94+
interacts: C1 · implemented: nameparser/_pipeline/_pieces.py
9595

9696
## Particles & surname prefixes (P)
9797

@@ -462,7 +462,7 @@ S2. Rationale: generational suffixes and credentials are recognized
462462
"Jack Wei Ma" → suffix="Ma"
463463
"Jack Wei Ma" → ambiguities=("suffix-or-name",)
464464
"Smith Jr." → family=""
465-
implemented: nameparser/_pipeline/_classify.py, nameparser/_pipeline/_group.py, nameparser/_pipeline/_vocab.py
465+
implemented: nameparser/_pipeline/_classify.py, nameparser/_pipeline/_group.py, nameparser/_pipeline/_pieces.py, nameparser/_pipeline/_vocab.py
466466

467467
S3. Rationale: credentials are often written run together with
468468
periods; the chunks between the periods are what carry the

nameparser/_pipeline/_assign.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
from nameparser._pipeline._vocab import (
4242
effective_script, is_suffix_lenient, resolve_script_set,
4343
)
44-
from nameparser._pipeline._group import (
44+
from nameparser._pipeline._pieces import (
4545
_is_suffix_piece, _leading_titles, _peel_trailing, _peel_walk,
4646
_segment_holds_no_name,
4747
)
@@ -60,7 +60,7 @@ def _set_roles(tokens: list[WorkToken], piece: tuple[int, ...],
6060
# rules.md#H2: "an abbreviation opening the part of the name that
6161
# carries the given name — the whole name, or the part after a
6262
# family comma — reads as a title even when unlisted" -- the count is
63-
# group's _leading_titles since #424 (its test, _is_leading_title, is
63+
# _pieces._leading_titles since #424 (its test, _is_leading_title, is
6464
# the leading-particle scan's too); the roles are set here.
6565
def _peel_leading_titles(pieces: tuple[tuple[int, ...], ...],
6666
ptags: tuple[frozenset[str], ...],
@@ -188,7 +188,7 @@ def _assign_main(seg_idx: int, state: ParseState,
188188
_set_roles(tokens, pieces[rest[0]], Role.FAMILY)
189189
return None
190190
# peel the trailing suffix run: k = first index in rest from which
191-
# every piece is a suffix. The walk is group's _peel_trailing since
191+
# every piece is a suffix. The walk is _pieces._peel_trailing since
192192
# #425 -- one walk, shared with the bound-given reserve, and
193193
# documented there. Every bare ambiguous acronym it had to resolve
194194
# is one coin-flip each, in either direction, so the report

0 commit comments

Comments
 (0)