From 61df6fa648ca9eefc0b34dddc5c16a1b8a69695e Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Sat, 4 Jul 2026 23:16:12 -0700 Subject: [PATCH 1/5] Stop parsing from mutating the Constants it reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit parse_pieces() and join_on_conjunctions() derived new lookup entries while parsing (period-joined titles/suffixes like "Lt.Gov.", conjunction-joined pieces like "Mr. and Mrs." and "von und zu") and add()ed them directly to self.C — by default the shared module-level CONSTANTS singleton. Parsing one name therefore permanently changed how every later name in the process parsed: results depended on input order, were not reproducible across runs, and concurrent parsing raced on the shared sets. It also churned the suffixes_prefixes_titles cache, which invalidated on every such add(). The derivations are only needed within the parse that produced them, so track them in per-instance _derived_* sets that the is_title / is_suffix / is_conjunction / is_prefix / is_rootname predicates consult alongside self.C, and reset them at the start of each parse_full_name() run. Parse results are unchanged (each parse re-derives its own entries); the config is now read-only during parsing. Co-Authored-By: Claude Fable 5 --- docs/release_log.rst | 1 + nameparser/parser.py | 83 ++++++++++++++++++++++++++++++----------- tests/test_constants.py | 73 ++++++++++++++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 21 deletions(-) diff --git a/docs/release_log.rst b/docs/release_log.rst index d446a46..363169a 100644 --- a/docs/release_log.rst +++ b/docs/release_log.rst @@ -1,6 +1,7 @@ Release Log =========== * 1.3.0 - Unreleased + - Fix parsing writing back into the ``Constants`` it reads (usually the shared module-level ``CONSTANTS``): pieces derived while parsing a name — period-joined titles/suffixes like ``"Lt.Gov."`` and conjunction-joined pieces like ``"Mr. and Mrs."`` or ``"von und zu"`` — are now tracked per parse instead of being permanently ``add()``-ed to the config, so parse results no longer depend on which names were parsed earlier in the process and parsing no longer mutates shared state across threads - Remove the vestigial ``unparsable`` attribute: the guard that was meant to set it has been unreachable since 2013 (v0.2.9), so it has reported ``False`` for every parsed name for over a decade; check ``len(name) == 0`` to detect an empty parse - Fix ``__hash__`` to lowercase the name like ``__eq__`` does, so equal ``HumanName`` instances hash equal and behave correctly in sets and dicts - Fix ``initials()`` emitting a stray empty initial (e.g. ``"J. . V."``) -- or raising ``TypeError`` when ``empty_attribute_default`` is ``None`` -- for name parts with no initialable words, e.g. a prefix-only middle name like ``"de la"`` diff --git a/nameparser/parser.py b/nameparser/parser.py index 0ee4f14..b391b39 100644 --- a/nameparser/parser.py +++ b/nameparser/parser.py @@ -115,6 +115,19 @@ def __init__( if type(self.C) is not type(CONSTANTS): self.C = Constants() + # Lookup entries derived while parsing this instance (period-joined + # titles/suffixes like "Lt.Gov.", conjunction-joined pieces like + # "Mr. and Mrs." or "von und zu"). Kept separate from self.C so that + # parsing never writes into the config — which is usually the shared + # module-level CONSTANTS — keeping results independent of what was + # parsed before and config reads safe across threads. Values are + # lc()-normalized, mirroring how SetManager stores them. Reset at the + # start of each parse_full_name() run. + self._derived_titles: set[str] = set() + self._derived_suffixes: set[str] = set() + self._derived_conjunctions: set[str] = set() + self._derived_prefixes: set[str] = set() + self.encoding = encoding self.string_format = string_format if string_format is not None else self.C.string_format self.initials_format = initials_format if initials_format is not None else self.C.initials_format @@ -144,6 +157,11 @@ def __setstate__(self, state: dict) -> None: if state.get('C') is None: state['C'] = CONSTANTS self.__dict__.update(state) + # pickles from before the per-parse derived sets existed lack them; + # backfill so the is_* predicates work without a re-parse + for attr in ('_derived_titles', '_derived_suffixes', + '_derived_conjunctions', '_derived_prefixes'): + self.__dict__.setdefault(attr, set()) def __iter__(self) -> Iterator[str]: return self @@ -550,8 +568,11 @@ def _set_list(self, attr: str, value: str | list[str] | None) -> None: # Parse helpers def is_title(self, value: str) -> bool: - """Is in the :py:data:`~nameparser.config.titles.TITLES` set.""" - return lc(value) in self.C.titles + """Is in the :py:data:`~nameparser.config.titles.TITLES` set or was + derived as a title earlier in this parse (e.g. ``"Lt.Gov."``, + ``"Mr. and Mrs."``).""" + word = lc(value) + return word in self.C.titles or word in self._derived_titles def is_leading_title(self, piece: str) -> bool: """ @@ -574,7 +595,9 @@ def is_conjunction(self, piece: str) -> bool: if self.is_conjunction(item): return True return False - return piece.lower() in self.C.conjunctions and not self.is_an_initial(piece) + return (piece.lower() in self.C.conjunctions + or piece.lower() in self._derived_conjunctions) \ + and not self.is_an_initial(piece) def is_prefix(self, piece: str) -> bool: """ @@ -586,7 +609,8 @@ def is_prefix(self, piece: str) -> bool: if self.is_prefix(item): return True return False - return lc(piece) in self.C.prefixes + word = lc(piece) + return word in self.C.prefixes or word in self._derived_prefixes def is_bound_first_name(self, piece: str) -> bool: """Lowercased, leading/trailing-periods-stripped version of piece is in :py:attr:`~nameparser.config.Constants.bound_first_names`.""" @@ -646,8 +670,10 @@ def is_suffix(self, piece: str) -> bool: return True return False else: - return ((lc(piece).replace('.', '') in self.C.suffix_acronyms) - or (lc(piece) in self.C.suffix_not_acronyms)) \ + word = lc(piece) + return ((word.replace('.', '') in self.C.suffix_acronyms) + or (word in self.C.suffix_not_acronyms) + or (word in self._derived_suffixes)) \ and not self.is_an_initial(piece) def are_suffixes(self, pieces: Iterable[str]) -> bool: @@ -671,7 +697,10 @@ def is_suffix_lenient(self, piece: str) -> bool: is_suffix() would otherwise reject. Only safe for pieces in unambiguous positions, e.g. after a comma ("John Ingram, V"). """ - return lc(piece) in self.C.suffix_not_acronyms or self.is_suffix(piece) + word = lc(piece) + return word in self.C.suffix_not_acronyms \ + or word in self._derived_suffixes \ + or self.is_suffix(piece) def expand_suffix_delimiter(self, part: str) -> list[str]: """Split a single post-comma part on :py:attr:`suffix_delimiter`, @@ -696,7 +725,11 @@ def is_rootname(self, piece: str) -> bool: """ Is not a known title, suffix or prefix. Just first, middle, last names. """ - return lc(piece) not in self.C.suffixes_prefixes_titles \ + word = lc(piece) + return word not in self.C.suffixes_prefixes_titles \ + and word not in self._derived_titles \ + and word not in self._derived_suffixes \ + and word not in self._derived_prefixes \ and not self.is_an_initial(piece) def is_an_initial(self, value: str) -> bool: @@ -1005,6 +1038,13 @@ def parse_full_name(self) -> None: self.nickname_list = [] self.maiden_list = [] + # each parse derives these from scratch; entries from a previous + # full_name must not influence this one + self._derived_titles = set() + self._derived_suffixes = set() + self._derived_conjunctions = set() + self._derived_prefixes = set() + self.pre_process() self._full_name = self.collapse_whitespace(self._full_name) @@ -1195,8 +1235,8 @@ def parse_pieces(self, parts: Iterable[str], additional_parts_count: int = 0) -> output += [s for s in (x.strip(' ,') for x in part.split(' ')) if s] # If part contains periods, check if it's multiple titles or suffixes - # together without spaces if so, add the new part with periods to the - # constants so they get parsed correctly later + # together without spaces. If so, register the periods-joined part as + # a derived title/suffix for this parse so it gets recognized later for part in output: # if this part has a period not at the beginning or end if self.C.regexes.period_not_at_end and self.C.regexes.period_not_at_end.match(part): @@ -1206,12 +1246,12 @@ def parse_pieces(self, parts: Iterable[str], additional_parts_count: int = 0) -> titles = list(filter(self.is_title, period_chunks)) suffixes = list(filter(self.is_suffix, period_chunks)) - # add the part to the constant so it will be found + # register the part so it will be found by the is_* checks if len(list(titles)): - self.C.titles.add(part) + self._derived_titles.add(lc(part)) continue if len(list(suffixes)): - self.C.suffix_not_acronyms.add(part) + self._derived_suffixes.add(lc(part)) continue return self.join_on_conjunctions(output, additional_parts_count) @@ -1226,10 +1266,11 @@ def join_on_conjunctions(self, pieces: list[str], additional_parts_count: int = ['The', 'Secretary', 'of', 'State', 'Hillary', 'Clinton'] ==> ['The Secretary of State', 'Hillary', 'Clinton'] - When joining titles, saves newly formed piece to the instance's titles - constant so they will be parsed correctly later. E.g. after parsing the - example names above, 'The Secretary of State' and 'Mr. and Mrs.' would - be present in the titles constant set. + When joining titles, registers the newly formed piece as a derived + title for the current parse so it will be recognized correctly later + in the same parse. E.g. while parsing the example names above, + 'The Secretary of State' and 'Mr. and Mrs.' are treated as titles. + The configuration in ``self.C`` is never modified. :param list pieces: name pieces strings after split on spaces :param int additional_parts_count: @@ -1259,8 +1300,8 @@ def join_on_conjunctions(self, pieces: list[str], additional_parts_count: int = for cont_i in reversed(contiguous_conj_i): new_piece = " ".join(pieces[cont_i[0]: cont_i[1]+1]) pieces[cont_i[0]:cont_i[1]+1] = [new_piece] - # add newly joined conjunctions to constants to be found later - self.C.conjunctions.add(new_piece) + # register newly joined conjunctions to be found later this parse + self._derived_conjunctions.add(lc(new_piece)) if len(pieces) == 1: # if there's only one piece left, nothing left to do @@ -1272,12 +1313,12 @@ def join_on_conjunctions(self, pieces: list[str], additional_parts_count: int = def register_joined_piece(new_piece: str, neighbor: str) -> None: if self.is_title(neighbor): # when joining to a title, make new_piece a title too - self.C.titles.add(new_piece) + self._derived_titles.add(lc(new_piece)) if self.is_prefix(neighbor): # when joining to a prefix, make new_piece a prefix too, so # e.g. "von" + "und" bridges into "von und" and can still # chain onto a following prefix/lastname (see "von und zu") - self.C.prefixes.add(new_piece) + self._derived_prefixes.add(lc(new_piece)) def shift_conj_index(past: int, by: int) -> None: # after removing pieces at/after `past`, indices of the diff --git a/tests/test_constants.py b/tests/test_constants.py index 27d8e5d..f7df1b7 100644 --- a/tests/test_constants.py +++ b/tests/test_constants.py @@ -487,6 +487,79 @@ def test_setstate_raises_on_missing_descriptor_field(self) -> None: restored.__setstate__(state) +class ParsingDoesNotMutateConfigTests(HumanNameTestBase): + """Parsing a name must never write back into the Constants it reads. + + The parser derives extra lookup entries while parsing (period-joined + titles/suffixes like "Lt.Gov.", conjunction-joined pieces like + "Mr. and Mrs." or "von und zu"). Those derivations are needed within the + parse, but historically they were add()ed to ``self.C`` — by default the + shared module-level CONSTANTS singleton — so parsing one name permanently + changed how every later name in the process parsed: parse results depended + on input order, and concurrent parsing raced on the shared sets. + """ + + _WATCHED_ATTRS = ('titles', 'prefixes', 'conjunctions', + 'suffix_acronyms', 'suffix_not_acronyms') + + def _assert_parse_leaves_config_unchanged(self, name: str) -> HumanName: + from nameparser.config import CONSTANTS + before = {a: set(getattr(CONSTANTS, a)) for a in self._WATCHED_ATTRS} + hn = HumanName(name) + for attr, snapshot in before.items(): + leaked = set(getattr(CONSTANTS, attr)) - snapshot + self.assertEqual(leaked, set(), + f"parsing {name!r} leaked {leaked!r} into CONSTANTS.{attr}") + return hn + + def test_period_joined_title_does_not_leak_into_titles(self) -> None: + hn = self._assert_parse_leaves_config_unchanged("Lt.Gov. John Doe") + # the within-parse derivation must still work + self.m(hn.title, "Lt.Gov.", hn) + self.m(hn.first, "John", hn) + self.m(hn.last, "Doe", hn) + + def test_period_joined_suffix_does_not_leak_into_suffixes(self) -> None: + hn = self._assert_parse_leaves_config_unchanged("John Doe JD.CPA") + self.m(hn.first, "John", hn) + self.m(hn.last, "Doe", hn) + self.m(hn.suffix, "JD.CPA", hn) + + def test_joined_conjunctions_do_not_leak_into_conjunctions(self) -> None: + hn = self._assert_parse_leaves_config_unchanged("Louis of the Netherlands") + self.m(hn.first, "Louis of the Netherlands", hn) + + def test_title_conjunction_join_does_not_leak_into_titles(self) -> None: + hn = self._assert_parse_leaves_config_unchanged("Mr. and Mrs. John Smith") + self.m(hn.title, "Mr. and Mrs.", hn) + self.m(hn.first, "John", hn) + self.m(hn.last, "Smith", hn) + + def test_prefix_conjunction_join_does_not_leak_into_prefixes(self) -> None: + hn = self._assert_parse_leaves_config_unchanged("Alois von und zu Liechtenstein") + self.m(hn.first, "Alois", hn) + self.m(hn.last, "von und zu Liechtenstein", hn) + + def test_instance_owned_constants_not_mutated_by_parsing(self) -> None: + hn = HumanName("", constants=None) + before = {a: set(getattr(hn.C, a)) for a in self._WATCHED_ATTRS} + hn.full_name = "Lt.Gov. John Doe" + for attr, snapshot in before.items(): + leaked = set(getattr(hn.C, attr)) - snapshot + self.assertEqual(leaked, set(), + f"parsing leaked {leaked!r} into the instance's own C.{attr}") + self.m(hn.title, "Lt.Gov.", hn) + + def test_derivations_reset_between_parses_of_same_instance(self) -> None: + # Re-assigning full_name re-parses; each parse must re-derive from a + # clean slate and still resolve the period-joined title. + hn = HumanName("Lt.Gov. John Doe") + hn.full_name = "Lt.Gov. Jane Roe" + self.m(hn.title, "Lt.Gov.", hn) + self.m(hn.first, "Jane", hn) + self.m(hn.last, "Roe", hn) + + class SuffixesPrefixesTitlesPerformanceTests(HumanNameTestBase): """Guard against accidental cache removal on suffixes_prefixes_titles. From 46d4a8dc98e0be4e6b18693bd77f936b2344b5e2 Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Sat, 4 Jul 2026 23:33:24 -0700 Subject: [PATCH 2/5] Address PR review findings - Fix stale parse_pieces docstring that still described adding derived parts "to the constant" - Document the derived-set branch in the is_prefix / is_suffix / is_conjunction docstrings, and reword is_leading_title's now-vacuous "does not mutate C.titles" contrast into the narrower load-bearing fact (the regex match is not registered anywhere, even per-parse) - Remove the redundant _derived_suffixes check from is_suffix_lenient: it falls through to is_suffix(), which already consults the overlay, and derived suffixes always contain an interior period so they can never trip the is_an_initial() veto that "lenient" exists to bypass - Add mutation-verified regression tests for the two coverage gaps: is_rootname's derived-title exclusion (its overlay checks were deletable without any test failing, yet dropping them misparses "Lt.Gov. juan e garcia" into an empty first name) and the __setstate__ backfill for pickles predating the derived sets (whose absence crashes capitalize() with AttributeError) Co-Authored-By: Claude Fable 5 --- nameparser/parser.py | 27 +++++++++++++++------------ tests/test_python_api.py | 21 +++++++++++++++++++++ tests/test_titles.py | 12 ++++++++++++ 3 files changed, 48 insertions(+), 12 deletions(-) diff --git a/nameparser/parser.py b/nameparser/parser.py index b391b39..8f1160e 100644 --- a/nameparser/parser.py +++ b/nameparser/parser.py @@ -582,14 +582,16 @@ def is_leading_title(self, piece: str) -> bool: ``is_an_initial()`` check, is what excludes single-letter initials like ``"J."``. Only meaningful for pieces in the title position (before the first name is set) — a period-abbreviation appearing - later in the name is left as a middle name. Does not mutate - ``C.titles``, so the periodless form (``"Major"``) is never affected - in later parses. + later in the name is left as a middle name. The match is not + registered in ``C.titles`` or the per-parse derived titles, so + matching ``"Major."`` here never makes ``"Major"`` (or ``"Major."``) + a recognized title elsewhere, even within the same parse. """ return self.is_title(piece) or bool(self.C.regexes.period_abbreviation.match(piece)) def is_conjunction(self, piece: str) -> bool: - """Is in the conjunctions set and not :py:func:`is_an_initial()`.""" + """Is in the conjunctions set — config or derived earlier in this + parse (e.g. ``"of the"``) — and not :py:func:`is_an_initial()`.""" if isinstance(piece, list): for item in piece: if self.is_conjunction(item): @@ -602,7 +604,8 @@ def is_conjunction(self, piece: str) -> bool: def is_prefix(self, piece: str) -> bool: """ Lowercased, leading/trailing-periods-stripped version of piece is in the - :py:data:`~nameparser.config.prefixes.PREFIXES` set. + :py:data:`~nameparser.config.prefixes.PREFIXES` set, or was derived as + a prefix earlier in this parse (e.g. ``"von und"``). """ if isinstance(piece, list): for item in piece: @@ -657,7 +660,9 @@ def is_roman_numeral(self, value: str) -> bool: def is_suffix(self, piece: str) -> bool: """ - Is in the suffixes set and not :py:func:`is_an_initial()`. + Is in the suffixes set — or was derived as a period-joined suffix + earlier in this parse (e.g. ``"JD.CPA"``) — and not + :py:func:`is_an_initial()`. Some suffixes may be acronyms (M.B.A) while some are not (Jr.), so we remove the periods from `piece` when testing against @@ -697,10 +702,7 @@ def is_suffix_lenient(self, piece: str) -> bool: is_suffix() would otherwise reject. Only safe for pieces in unambiguous positions, e.g. after a comma ("John Ingram, V"). """ - word = lc(piece) - return word in self.C.suffix_not_acronyms \ - or word in self._derived_suffixes \ - or self.is_suffix(piece) + return lc(piece) in self.C.suffix_not_acronyms or self.is_suffix(piece) def expand_suffix_delimiter(self, part: str) -> list[str]: """Split a single post-comma part on :py:attr:`suffix_delimiter`, @@ -1211,8 +1213,9 @@ def parse_pieces(self, parts: Iterable[str], additional_parts_count: int = 0) -> lastname prefixes. Tokens that are empty after stripping spaces and commas are dropped, so the returned pieces never contain empty strings. If parts have periods in the middle, try splitting - on periods and check if the parts are titles or suffixes. If they are - add to the constant so they will be found. + on periods and check if the parts are titles or suffixes. If they are, + register the periods-joined part as a derived title/suffix for this + parse so it will be recognized; the constants are not modified. :param list parts: name part strings from the comma split :param int additional_parts_count: diff --git a/tests/test_python_api.py b/tests/test_python_api.py index aaf4661..741c0f1 100644 --- a/tests/test_python_api.py +++ b/tests/test_python_api.py @@ -95,6 +95,27 @@ def test_name_instance_deepcopy_isolates_instance_config(self) -> None: self.assertIn('chancellor', dup.C.titles) self.assertNotIn('marker', hn.C.titles) + def test_unpickle_legacy_state_without_derived_sets(self) -> None: + """Pickles from before the per-parse derived sets existed must still work. + + Their state lacks the ``_derived_*`` attributes, which the ``is_*`` + predicates (and through them ``capitalize()``) read directly, so + ``__setstate__`` must backfill them rather than crash with + AttributeError on first use. + """ + hn = HumanName("dr. juan de la vega jr.") + legacy_state = { + k: v for k, v in hn.__getstate__().items() + if not k.startswith('_derived_') + } + + restored = HumanName.__new__(HumanName) + restored.__setstate__(legacy_state) + + self.assertTrue(restored.is_title('dr.')) + restored.capitalize() # reads _derived_prefixes via cap_word/is_prefix + self.assertEqual(str(restored), "Dr. Juan de la Vega Jr.") + def test_pickle_default_name_preserves_singleton_identity(self) -> None: """A default HumanName must re-attach to CONSTANTS after a pickle round-trip. diff --git a/tests/test_titles.py b/tests/test_titles.py index 0a95081..56c066c 100644 --- a/tests/test_titles.py +++ b/tests/test_titles.py @@ -225,6 +225,18 @@ def test_title_with_periods_lastname_comma(self) -> None: self.m(hn.first, "John", hn) self.m(hn.last, "Doe", hn) + def test_title_with_periods_and_single_letter_middle_name(self) -> None: + # A derived title ("Lt.Gov.") must be excluded from the rootname + # count that join_on_conjunctions() uses for its single-letter + # conjunction heuristic. If is_rootname() misses the derived titles, + # the count reaches 4 and "e" is treated as a conjunction, joining + # "juan e garcia" into a single last-name piece with no first name. + hn = HumanName("Lt.Gov. juan e garcia") + self.m(hn.title, "Lt.Gov.", hn) + self.m(hn.first, "juan", hn) + self.m(hn.middle, "e", hn) + self.m(hn.last, "garcia", hn) + def test_mac_with_spaces(self) -> None: hn = HumanName("Jane Mac Beth") self.m(hn.first, "Jane", hn) From 268b19267785c59504035686e1c84b24e53b5937 Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Sat, 4 Jul 2026 23:42:05 -0700 Subject: [PATCH 3/5] Document that parsing never modifies the shared config The shared-configuration warning in customize.rst now has a precise boundary worth stating: after the parse-time mutation fix, instances can only affect each other through explicit add/remove calls, so concurrent parsing against the shared CONSTANTS is safe. Co-Authored-By: Claude Fable 5 --- docs/customize.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/customize.rst b/docs/customize.rst index b344eb0..6eba656 100644 --- a/docs/customize.rst +++ b/docs/customize.rst @@ -429,6 +429,9 @@ When you modify the configuration, by default this will modify the behavior all HumanName instances. This could be a handy way to set it up for your entire project, but it could also lead to some unexpected behavior because changing the config on one instance could modify the behavior of another instance. +Parsing itself never modifies the configuration — only your own ``add`` and +``remove`` calls do — so the shared instance is safe to read concurrently, +e.g. parsing names on multiple threads. .. doctest:: module config :options: +ELLIPSIS, +NORMALIZE_WHITESPACE From 3ce83149c2f399f9d4e79f958ff7d477e0e99616 Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Sun, 5 Jul 2026 12:07:47 -0700 Subject: [PATCH 4/5] Make the no-mutation leak tests self-maintaining; document the invariant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the hand-enumerated _WATCHED_ATTRS list with a structural snapshot derived from Constants.__getstate__(), the canonical listing of an instance's own config. A collection added to Constants later is watched automatically — there is no list to remember to update (unlike conftest's _COLLECTION_CONFIG_ATTRS, which remains the only manual one). A sanity assert fails loud if the discovery ever stops covering the five historically-mutated sets, and the snapshot now also covers TupleManager collections and scalar overrides. Update AGENTS.md: fix the two Architecture lines that still described parse_pieces()/join_on_conjunctions() as writing into the constants, and add a gotcha documenting the parsing-never-writes-config invariant and the _derived_* overlay recipe for future derived categories. Co-Authored-By: Claude Fable 5 --- AGENTS.md | 8 ++++--- tests/test_constants.py | 52 +++++++++++++++++++++++++++++++---------- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 152a9ae..4c0d98c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -94,9 +94,9 @@ Each module defines a plain Python set of known name pieces: Parse flow: 1. `pre_process()` — strips nicknames/maiden names (parenthesis/quotes, routed to `nickname_list`/`maiden_list` per `Constants.nickname_delimiters`/`maiden_delimiters`) and emoji, fixes "Ph.D." variant spellings 2. Split on commas → 1 part (no comma), 2 parts (suffix-comma or lastname-comma), 3+ parts -3. `parse_pieces()` — splits on spaces, detects dotted abbreviations like "Lt.Gov." and adds them to constants dynamically +3. `parse_pieces()` — splits on spaces, detects dotted abbreviations like "Lt.Gov." and registers them as per-parse derived titles/suffixes (the `_derived_*` sets — never the config; see Gotchas) 4. `join_on_conjunctions()` — merges pieces adjacent to conjunctions into single tokens (e.g. `['Secretary', 'of', 'State']` → `['Secretary of State']`); also joins prefix particles to the following lastname token - — a piece merged with a title *or prefix* neighbor is re-registered into that constant set so later steps still recognize it, e.g. `von`+`und`+`zu` → registered as a prefix so the whole phrase joins to the last name (German "von und zu"; PR #191) + — a piece merged with a title *or prefix* neighbor is re-registered into the per-parse derived title/prefix set so later steps still recognize it, e.g. `von`+`und`+`zu` → registered as a derived prefix so the whole phrase joins to the last name (German "von und zu"; PR #191) 4a. `_join_bound_first_name()` — called immediately after step 4 in all three paths that build a first-name-bearing token sequence: no-comma, lastname-comma (post-comma pieces), and suffix-comma (`parts[0]`); merges bound given-name prefixes (e.g. "abdul") with the next piece before the assignment loop runs; suffixes are still in `pieces` at this point, so the `reserve_last` guard must count non-suffix pieces only. Not called for the lastname portion itself (`lastname_pieces` in the lastname-comma path) — that token sequence is a surname, not first-name text, so the join must not apply there. The join lives at each of these three call sites individually rather than inside `parse_pieces()` itself. That's because `parse_pieces()` is also called for the lastname portion with the same `additional_parts_count` value used for the (joinable) post-comma given-names portion, so `additional_parts_count` alone can't disambiguate which callers want the join. 5. Iterates pieces, assigning to `title_list`, `first_list`, `middle_list`, `last_list`, `suffix_list` 6. `post_process()` — `handle_firstnames()` swaps first/last when only a title + one name; then, gated on `patronymic_name_order`, `handle_east_slavic_patronymic_name_order()` and `handle_turkic_patronymic_name_order()` reorder Russian-formal-order and reversed Turkic patronymics; then, gated on `middle_name_as_last`, `handle_middle_name_as_last()` folds `middle_list` into `last_list`; finally `handle_capitalization()` applies optional auto-cap. Any new `self._attr` used by `post_process()` helpers must be initialized in `__init__` (with its default value) — the direct-kwargs path bypasses `parse_full_name()`, so the attribute won't exist otherwise. @@ -112,7 +112,7 @@ Each named attribute (`title`, `first`, etc.) is a `@property` that joins its co 4. conftest auto-restores scalar CONSTANTS between tests, but tests that *set* CONSTANTS mid-run still need their own try/finally 5. Update `docs/customize.rst`'s constants list (and `docs/usage.rst` if it affects a documented example) — don't wait for the release checklist, these `.rst` docs aren't covered by CI so a stale one can ship silently. Check `AGENTS.md` itself too (Extension Patterns, Gotchas, the Architecture section) for now-stale attribute/test names or descriptions — it has the same blind spot as the `.rst` docs and the release checklist only catches it at release time. -**Adding a new mutable/collection `Constants` attribute** (a `SetManager`/`TupleManager`-backed group, e.g. `nickname_delimiters`/`maiden_delimiters`): add it to `_COLLECTION_CONFIG_ATTRS` in `tests/conftest.py`, or tests that mutate the global `CONSTANTS` copy will leak state into later tests. Contents must be deep-copyable (the snapshot uses `copy.deepcopy`) — already true for the existing manager types. +**Adding a new mutable/collection `Constants` attribute** (a `SetManager`/`TupleManager`-backed group, e.g. `nickname_delimiters`/`maiden_delimiters`): add it to `_COLLECTION_CONFIG_ATTRS` in `tests/conftest.py`, or tests that mutate the global `CONSTANTS` copy will leak state into later tests. Contents must be deep-copyable (the snapshot uses `copy.deepcopy`) — already true for the existing manager types. (The parse-no-mutation tests in `tests/test_constants.py` need no such registration — they discover collections structurally via `Constants.__getstate__()`; conftest's list is the only manual one.) Add a dedicated `copy.deepcopy()` round-trip test for it too (see `test_regexes_deepcopy_roundtrip`/`test_nickname_delimiters_deepcopy_roundtrip` in `tests/test_constants.py`), not just reliance on conftest's autouse snapshot/restore exercising it incidentally. `TupleManager`/`RegexTupleManager.__getattr__` answer *any* unknown attribute lookup — including dunder probes like `__deepcopy__` — so a new manager subtype or a `__getattr__` tweak can silently break `copy.deepcopy` (this bit `RegexTupleManager` before the dunder-lookup guard was added). A direct test on the new attribute's own manager instance catches that where the conftest fixture, which never asserts on the copy, would not. As with the scalar-attribute pattern above, also check `AGENTS.md` itself for now-stale references when you touch this. @@ -128,6 +128,8 @@ Add a dedicated `copy.deepcopy()` round-trip test for it too (see `test_regexes_ ## Gotchas +**Parsing must never write into `Constants`** — parse-time derived recognition (dotted abbreviations like "Lt.Gov.", conjunction-joined titles/prefixes like "Mr. and Mrs." / "von und zu") lives in per-instance `HumanName._derived_titles` / `_derived_suffixes` / `_derived_conjunctions` / `_derived_prefixes` sets, consulted by `is_title`/`is_suffix`/`is_conjunction`/`is_prefix`/`is_rootname`, reset at the start of `parse_full_name()`, and backfilled in `__setstate__` for pre-existing pickles. Never `self.C..add()` during parsing — `self.C` is usually the shared `CONSTANTS` singleton, so a write makes parse results order-dependent and thread-unsafe (this was a real bug through 1.2.x). `ParsingDoesNotMutateConfigTests` (`tests/test_constants.py`) enforces the invariant by snapshotting the whole config around a parse; it discovers collections structurally via `Constants.__getstate__()`, so new `Constants` collections are watched automatically — nothing to register. If a new derived category is ever needed: add a `_derived_*` set in `__init__`, reset it in `parse_full_name()`, backfill it in `__setstate__`, consult it in the matching `is_*` predicate (store `lc()`-normalized values, mirroring `SetManager`), and add a leak test with a name that triggers it. + **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. **`is_leading_title()` infers titles beyond the `TITLES` set, but only in leading position** — an unrecognized multi-letter word ending in a single trailing period (matched via the `period_abbreviation` regex, `{2,}` letters) is treated as a title when it appears before the first name is set, e.g. `"Major. Dona Smith"` → `title='Major.'`. It's distinct from `is_title()` and does not mutate `C.titles`, so the periodless form (`"Major"`) is unaffected elsewhere. The `{2,}` length requirement — not a separate initials check — is what excludes single-letter initials like `"J."` from being swallowed as titles; the same word after the first name is left as a middle name. (#109; see `docs/usage.rst` "Leading Period-Abbreviation Titles") diff --git a/tests/test_constants.py b/tests/test_constants.py index f7df1b7..1a415d3 100644 --- a/tests/test_constants.py +++ b/tests/test_constants.py @@ -499,17 +499,48 @@ class ParsingDoesNotMutateConfigTests(HumanNameTestBase): on input order, and concurrent parsing raced on the shared sets. """ - _WATCHED_ATTRS = ('titles', 'prefixes', 'conjunctions', - 'suffix_acronyms', 'suffix_not_acronyms') + @staticmethod + def _config_snapshot(constants: Constants) -> dict: + """Snapshot every piece of configuration ``constants`` owns. + + Enumerated via ``Constants.__getstate__()`` — the canonical listing of + an instance's own config (descriptor-backed names mapped to their + public form, private caches like ``_pst`` excluded) — so a collection + added to ``Constants`` later is watched automatically, with no + attribute list to keep in sync. + """ + snap = {} + for attr, value in constants.__getstate__().items(): + if isinstance(value, SetManager): + snap[attr] = set(value) + elif isinstance(value, TupleManager): + snap[attr] = dict(value) + else: + snap[attr] = value # scalar override + # Fail loud if the structural discovery ever stops seeing the sets + # the parser historically leaked into. + assert {'titles', 'prefixes', 'conjunctions', + 'suffix_acronyms', 'suffix_not_acronyms'} <= set(snap), \ + "config snapshot no longer covers the historically-mutated sets" + return snap + + def _assert_config_unchanged(self, constants: Constants, before: dict, parsed: str) -> None: + after = self._config_snapshot(constants) + diffs = [] + for attr in sorted(set(before) | set(after)): + b, a = before.get(attr), after.get(attr) + if b != a: + if isinstance(b, set) and isinstance(a, set): + diffs.append(f"{attr}: added {sorted(a - b)}, removed {sorted(b - a)}") + else: + diffs.append(f"{attr}: {b!r} -> {a!r}") + self.assertEqual(diffs, [], f"parsing {parsed!r} changed the config: {diffs}") def _assert_parse_leaves_config_unchanged(self, name: str) -> HumanName: from nameparser.config import CONSTANTS - before = {a: set(getattr(CONSTANTS, a)) for a in self._WATCHED_ATTRS} + before = self._config_snapshot(CONSTANTS) hn = HumanName(name) - for attr, snapshot in before.items(): - leaked = set(getattr(CONSTANTS, attr)) - snapshot - self.assertEqual(leaked, set(), - f"parsing {name!r} leaked {leaked!r} into CONSTANTS.{attr}") + self._assert_config_unchanged(CONSTANTS, before, name) return hn def test_period_joined_title_does_not_leak_into_titles(self) -> None: @@ -542,12 +573,9 @@ def test_prefix_conjunction_join_does_not_leak_into_prefixes(self) -> None: def test_instance_owned_constants_not_mutated_by_parsing(self) -> None: hn = HumanName("", constants=None) - before = {a: set(getattr(hn.C, a)) for a in self._WATCHED_ATTRS} + before = self._config_snapshot(hn.C) hn.full_name = "Lt.Gov. John Doe" - for attr, snapshot in before.items(): - leaked = set(getattr(hn.C, attr)) - snapshot - self.assertEqual(leaked, set(), - f"parsing leaked {leaked!r} into the instance's own C.{attr}") + self._assert_config_unchanged(hn.C, before, "Lt.Gov. John Doe") self.m(hn.title, "Lt.Gov.", hn) def test_derivations_reset_between_parses_of_same_instance(self) -> None: From f7e679dd5dc122aaefaf5af445ce023821414a0c Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Sun, 5 Jul 2026 12:43:59 -0700 Subject: [PATCH 5/5] Add a Workflow nudge to re-read AGENTS.md before opening a PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing self-staleness reminders live inside two extension-pattern recipes and the release checklist, so a behavior change that fits neither pattern reaches PR time with no check on this file — exactly how the parse-flow lines describing the old mutate-the-config behavior survived the PR #222 review round. The nudge states why grep is the wrong tool for it: AGENTS.md paraphrases behavior in its own words, so stale text rarely matches the diff's phrasing. Co-Authored-By: Claude Fable 5 --- AGENTS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 4c0d98c..6405df3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,6 +7,8 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co For non-trivial changes, use a feature branch and open a PR. Branch naming: `fix/issue-NNN-short-description` or `feat/short-description`. +Before opening the PR, if the change alters parser behavior or internals, *read* the Architecture, Extension Patterns, and Gotchas sections of this file against the change — don't grep for it: AGENTS.md paraphrases behavior in its own words, so text made stale by a code change rarely matches the code's phrasing (a doc-staleness sweep driven by grep terms from the diff will miss it every time). The same applies when scoping a doc-review pass or a subagent prompt: include AGENTS.md in the list of docs to check, or it won't be checked. + ## Commands ```bash