Fix bare string to SetManager silently shredding into characters (#240)

* Fix bare string to SetManager silently shredding into characters

Every set-backed Constants argument is annotated Iterable[str], which a
bare str satisfies — as an iterable of its characters. SetManager's
set(elements) then silently replaced the whole default set with single
characters (Constants(titles='dr') -> titles == {'d', 'r'}), producing
wrong parses with no error anywhere. The type system cannot catch this,
so it must be a runtime check.

Reject str and bytes in SetManager.__init__ with a TypeError that
includes the wrap-it-in-a-list fix, covering all nine set-backed
Constants arguments and direct SetManager construction in one place.
All internal call sites pass module-level tuples/sets, and pickling
restores SetManager by state without re-running __init__, so only the
buggy input path is affected.

Closes #238

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Review follow-ups: guard set operators and fix the bytes hint

The Set mixin's __or__/__and__ hand _from_iterable a generator, so the
new __init__ guard never saw a bare-string operand: c.titles |= 'esq'
still silently added 'e', 's', 'q' as titles, while __sub__/__xor__
raised only by accident of constructing from the operand. Route all
four operators (and reflections) through a shared bare-string check.
typeshed declares narrower AbstractSet operands and omits the runtime
ABC's __rsub__, hence the targeted type ignores.

The bytes branch of the error hint was itself a trap: following "wrap
it in a list: [b'dr']" produces a set whose bytes elements never match
parsed str tokens — another silent failure. bytes now get "decode it
first: [b'dr'.decode()]".

Also correct the SetManager class docstring's "Only special
functionality" claim (the guards are user-visible API now) and drop
add()'s false "Can pass a list of strings" (that raises
AttributeError).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Normalize SetManager operator operands like add() does

The Set mixin operators build results from raw operand elements and
test them against the stored (normalized) ones, so even with the
bare-string guard, (titles | ['Esq.']) kept a raw 'Esq.' the parser's
lc() lookups could never match, and (titles & ['Dr.']) missed 'dr' —
the same silently-broken-config family as #238, one step from the
guarded case. Operands now pass through lc() in the existing operator
overrides, so operator results obey the same normalized-elements
invariant as add()-built sets. Idempotent for already-normalized
operands, so the internal suffixes_prefixes_titles union is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Normalize SetManager constructor elements and validate element types

Review follow-up on the operand-normalization commit: the constructor
still stored raw elements, which that commit turned from a latent
inconsistency into active misfires — SetManager(['Dr.']) & ['Dr.']
returned empty and - silently no-opped against the exact spelling
stored in the set, Constants(titles=[...]) remained the last
silently-dead config path (raw 'Chemistry' can never match the
parser's lc() lookups), and titles vs the suffixes_prefixes_titles
union disagreed about the same token.

Fold the operand helper into _normalized_elements, shared by __init__
and all operators, so every entry is stored in the form lookups expect.
Junk elements now raise a curated TypeError instead of failing
cryptically inside lc() (bytes, int) or being silently transmuted
(None became '').

Also scope the docstring claim that operators normalize "the same way
add() does" (add() additionally decodes bytes), correct "strips
periods" to leading/trailing in prose, and pin __rxor__ separately.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Simplify and speed up SetManager normalization paths

The operand-normalization work left a compounding perf regression:
operators delegated to the ABC mixins, whose _from_iterable re-entered
__init__ and re-validated the entire result element by element, and
Constants() re-checked ~1,400 already-normalized default constants on
every construction. Measured on the headline per-instance-config path,
HumanName(constants=None) was 6.8x slower than master (101 -> 690us)
and Constants() 21.6x slower (7.5 -> 162us).

Build operator results with plain set ops on already-normalized
elements via a private _from_normalized constructor (also dropping the
super()-delegation type ignores), short-circuit SetManager operands in
_normalized_elements (a SetManager is normalized by construction), and
validate the nine default config sets once at import, copied on
identity-checked defaults. Back to master parity: Constants() 8.2us,
HumanName(constants=None) 58us.

Also fold the single-caller _reject_bare_string into
_normalized_elements, note the deliberate bytes-policy divergence from
add_with_encoding(), merge two tests that pinned the same |= parse
end-to-end, trim the triple-stated operator rationale, and update the
stale AGENTS.md SetManager normalization note.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Review follow-ups: document mutation caveat, harden _from_normalized docs, add tests

- Document that the _DEFAULT_* identity fast path in Constants.__init__
  snapshots module constants at import time, so mutating a raw constant
  (e.g. TITLES.add(...)) after import isn't picked up by later
  Constants() instances — only the documented CONSTANTS.titles.add(...)
  path is supported.
- Strengthen _from_normalized's docstring to state explicitly that it
  performs no validation and callers must pre-normalize elements.
- Add tests pinning __rsub__'s operand order, confirming Constants()
  instances don't alias mutable state via the identity fast path, and
  confirming an equal-but-not-identical titles list still validates
  fully instead of false-matching the fast path.

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: derek73

AGENTS.md

@@ -85,7 +85,7 @@ Most modules define a plain Python set of known name pieces; `capitalization.py`
 
 `config/__init__.py` wraps everything into `SetManager` and `TupleManager` instances inside a `Constants` class. A module-level singleton `CONSTANTS` is shared across all `HumanName` instances by default.
 
-**Two-tier config pattern**: `CONSTANTS` is global; passing `None` as the second arg to `HumanName` creates a fresh per-instance `Constants()`. After modifying per-instance config you must call `hn.parse_full_name()` again. `SetManager.add()`/`remove()` normalizes inputs to lowercase with no periods, so callers don't need to worry about case.
+**Two-tier config pattern**: `CONSTANTS` is global; passing `None` as the second arg to `HumanName` creates a fresh per-instance `Constants()`. After modifying per-instance config you must call `hn.parse_full_name()` again. `SetManager` normalizes elements (lowercase, leading/trailing periods stripped) at every entry point — constructor, `add()`/`remove()`, and set operators — and raises `TypeError` for bare strings/bytes and non-str elements, so callers don't need to worry about case.
 
 **`_CachedUnionMember` descriptor**: The four PST-contributing attrs (`prefixes`, `suffix_acronyms`, `suffix_not_acronyms`, `titles`) are managed by this descriptor, which stores their values under the *private* name (`_prefixes`, `_titles`, etc.) in the instance `__dict__` so that the descriptor's `__set__` owns every assignment and can wire the cache-invalidation callback. Any code that inspects `__dict__` directly (e.g. `__getstate__`) must map `_xxx` → `xxx` for descriptor-managed attrs rather than filtering on `not k.startswith('_')`.
 

docs/release_log.rst

@@ -1,6 +1,8 @@
 Release Log
 ===========
 * 1.3.0 - Unreleased
+    - Fix a bare string passed to a set-backed ``Constants`` argument (e.g. ``Constants(titles='dr')``), to ``SetManager``, or as a ``SetManager`` set-operator operand (e.g. ``constants.titles |= 'esq'``) being silently split into single characters, replacing or polluting the set and producing wrong parses with no error; it now raises ``TypeError`` with the suggested fix — wrap strings in a list, decode ``bytes`` first (closes #238)
+    - Fix ``SetManager`` set operators and the constructor skipping the lowercase/strip-edge-periods normalization that ``add()`` applies: ``constants.titles |= ['Esq.']`` kept a raw ``'Esq.'`` the parser's lookups could never match, ``titles & ['Dr.']`` missed ``'dr'``, and ``Constants(titles=[...])`` stored raw elements that silently never matched; elements and operands are now normalized everywhere, and non-``str`` elements (``bytes``, ``None``, numbers) raise ``TypeError`` instead of crashing cryptically or being coerced
     - Fix the ``constants`` constructor argument silently discarding ``Constants`` *subclass* instances: the exact-type check replaced them with fresh defaults, throwing away the caller's configuration. Subclass instances are now used as given; anything that is neither ``None`` nor a ``Constants`` instance now raises ``TypeError`` instead of being silently swapped for defaults (closes #226)
     - Fix ``IndexError`` in ``initials()``/``initials_list()`` when a ``*_list`` attribute was assigned directly with an element containing unnormalized whitespace (e.g. ``name.middle_list = ['Q  R']``), bypassing the parser's whitespace normalization (closes #232)
     - Fix ``HumanName`` acting as its own iterator with a stored cursor: breaking out of a loop, iterating in nested loops, or calling ``len(name)`` mid-loop corrupted subsequent iteration; ``iter(name)`` now returns a fresh independent iterator each time. ``next(name)`` on the instance itself (undocumented) now raises ``TypeError`` — call ``next(iter(name))`` instead (closes #225)

nameparser/config/__init__.py

@@ -56,17 +56,77 @@ class SetManager(Set):
     Easily add and remove config variables per module or instance. Subclass of
     ``collections.abc.Set``.
 
-    Only special functionality beyond that provided by set() is
-    to normalize constants for comparison (lower case, no periods)
-    when they are add()ed and remove()d and allow passing multiple 
-    string arguments to the :py:func:`add()` and :py:func:`remove()` methods.
+    Special functionality beyond that provided by set() is to normalize
+    constants for comparison (lowercase, leading/trailing periods stripped)
+    when they are add()ed and remove()d, and to allow passing multiple
+    string arguments to the :py:func:`add()` and :py:func:`remove()`
+    methods. The constructor and the set operators apply the same
+    normalization to their elements and operands, so every entry is stored
+    in the form the parser's lookups expect, and they reject a bare string
+    with ``TypeError``, since e.g. ``set('dr')`` would silently build a set
+    of single characters.
 
     '''
 
     _on_change: Callable[[], None] | None
 
+    @classmethod
+    def _normalized_elements(cls, elements: Iterable[str]) -> set[str]:
+        # a SetManager's elements were validated and normalized when it was
+        # built, so copy them instead of re-validating — this is what keeps
+        # chained unions (suffixes_prefixes_titles) and default Constants()
+        # construction from re-checking ~1,400 entries per step
+        if isinstance(elements, SetManager):
+            return set(elements.elements)
+        # a bare string is an iterable of its characters, so set(value)
+        # would silently build a set of single characters — and bytes
+        # iterates to ints, which can never match parsed tokens (#238)
+        if isinstance(elements, bytes):
+            raise TypeError(
+                "expected an iterable of strings, got a single bytes; "
+                f"decode it first: [{elements!r}.decode()]"
+            )
+        if isinstance(elements, str):
+            raise TypeError(
+                "expected an iterable of strings, got a single str; "
+                f"wrap it in a list: [{elements!r}]"
+            )
+        # apply the same lc() normalization (lowercase, strip leading/
+        # trailing periods) that add() applies, and reject junk elements:
+        # lc() on bytes or int crashes without naming the culprit, and
+        # lc(None) silently transmutes to ''. Divergence from add() is
+        # deliberate: add_with_encoding() decodes bytes for back-compat,
+        # bulk boundaries stay strict.
+        normalized = set()
+        for s in elements:
+            if isinstance(s, bytes):
+                raise TypeError(
+                    f"expected str elements, got bytes; decode it first: {s!r}.decode()"
+                )
+            if not isinstance(s, str):
+                raise TypeError(
+                    f"expected str elements, got {type(s).__name__}: {s!r}"
+                )
+            normalized.add(lc(s))
+        return normalized
+
+    @classmethod
+    def _from_normalized(cls, elements: set[str]) -> 'SetManager':
+        # Private fast constructor: bypasses __init__ so results aren't
+        # re-validated element by element. This performs NO validation or
+        # normalization of `elements` -- the caller is fully responsible
+        # for guaranteeing every element is already a str that has passed
+        # through lc(). Only call this with a set built from other
+        # SetManagers' already-normalized .elements (operator results,
+        # prebuilt default copies); passing anything else silently defeats
+        # the constructor's #238 guarantees with no error raised here.
+        obj = cls.__new__(cls)
+        obj.elements = elements
+        obj._on_change = None
+        return obj
+
     def __init__(self, elements: Iterable[str]) -> None:
-        self.elements = set(elements)
+        self.elements = self._normalized_elements(elements)
         # Optional invalidation hook, wired by an owning Constants so that
         # in-place add()/remove() can clear its cached suffixes_prefixes_titles
         # union. None when the manager is used standalone.
@@ -90,6 +150,36 @@ def __contains__(self, value: object) -> bool:
     def __len__(self) -> int:
         return len(self.elements)
 
+    # The ABC mixins compare raw operand elements against stored (normalized)
+    # ones, and their __or__/__and__ accept a bare str as Iterable, so every
+    # operand is validated and normalized here. Results are built with plain
+    # set ops on already-normalized elements instead of delegating to the
+    # mixins, whose _from_iterable would re-validate the whole result
+    # through __init__.
+    #
+    # the runtime ABC accepts any Iterable operand, so annotate honestly and
+    # ignore typeshed's narrower AbstractSet declarations
+    def __or__(self, other: Iterable[str]) -> 'SetManager':  # type: ignore[override]
+        return self._from_normalized(self.elements | self._normalized_elements(other))
+
+    __ror__ = __or__
+
+    def __and__(self, other: Iterable[str]) -> 'SetManager':  # type: ignore[override]
+        return self._from_normalized(self.elements & self._normalized_elements(other))
+
+    __rand__ = __and__
+
+    def __sub__(self, other: Iterable[str]) -> 'SetManager':  # type: ignore[override]
+        return self._from_normalized(self.elements - self._normalized_elements(other))
+
+    def __rsub__(self, other: Iterable[str]) -> 'SetManager':
+        return self._from_normalized(self._normalized_elements(other) - self.elements)
+
+    def __xor__(self, other: Iterable[str]) -> 'SetManager':  # type: ignore[override]
+        return self._from_normalized(self.elements ^ self._normalized_elements(other))
+
+    __rxor__ = __xor__
+
     def add_with_encoding(self, s: str, encoding: str | None = None) -> None:
         """
         Add the lowercased, leading/trailing-periods-stripped version of the string to the set. Pass an
@@ -111,7 +201,7 @@ def add_with_encoding(self, s: str, encoding: str | None = None) -> None:
     def add(self, *strings: str) -> Self:
         """
         Add the lowercased, leading/trailing-periods-stripped version of the string arguments to the set.
-        Can pass a list of strings. Returns ``self`` for chaining.
+        Returns ``self`` for chaining.
         """
         for s in strings:
             self.add_with_encoding(s)
@@ -153,6 +243,31 @@ def _is_dunder(attr: str) -> bool:
     return attr.startswith("__") and attr.endswith("__")
 
 
+# The default config sets are module constants that never change, so
+# validate and normalize each one exactly once at import. Constants()
+# copies these via _normalized_elements' SetManager fast path instead of
+# re-checking ~1,400 elements per construction — a cost that otherwise
+# repeats on the per-instance-config path, HumanName(constants=None).
+#
+# This snapshot is taken once, at import time: mutating a raw constant
+# (e.g. `TITLES.add('x')`) after import is *not* picked up by Constants()
+# built afterward, since the identity check in Constants.__init__ reuses
+# this frozen SetManager rather than re-wrapping the (now-changed) raw
+# set. That's a behavior change from re-wrapping every time, but the
+# documented customization path mutates the SetManager wrapper on a
+# Constants instance (``CONSTANTS.titles.add(...)``), not the raw
+# constant, so this only affects an unsupported/undocumented pattern.
+_DEFAULT_PREFIXES = SetManager(PREFIXES)
+_DEFAULT_SUFFIX_ACRONYMS = SetManager(SUFFIX_ACRONYMS)
+_DEFAULT_SUFFIX_NOT_ACRONYMS = SetManager(SUFFIX_NOT_ACRONYMS)
+_DEFAULT_SUFFIX_ACRONYMS_AMBIGUOUS = SetManager(SUFFIX_ACRONYMS_AMBIGUOUS)
+_DEFAULT_TITLES = SetManager(TITLES)
+_DEFAULT_FIRST_NAME_TITLES = SetManager(FIRST_NAME_TITLES)
+_DEFAULT_CONJUNCTIONS = SetManager(CONJUNCTIONS)
+_DEFAULT_BOUND_FIRST_NAMES = SetManager(BOUND_FIRST_NAMES)
+_DEFAULT_NON_FIRST_NAME_PREFIXES = SetManager(NON_FIRST_NAME_PREFIXES)
+
+
 class TupleManager(dict[str, T]):
     '''
     A dictionary with dot.notation access. Subclass of ``dict``. Wraps the
@@ -487,15 +602,18 @@ def __init__(self,
         # These four descriptor assignments call _CachedUnionMember.__set__, which
         # calls _invalidate_pst() and establishes self._pst. They must come before
         # any read of suffixes_prefixes_titles.
-        self.prefixes = SetManager(prefixes)
-        self.suffix_acronyms = SetManager(suffix_acronyms)
-        self.suffix_not_acronyms = SetManager(suffix_not_acronyms)
-        self.titles = SetManager(titles)
-        self.first_name_titles = SetManager(first_name_titles)
-        self.conjunctions = SetManager(conjunctions)
-        self.bound_first_names = SetManager(bound_first_names)
-        self.non_first_name_prefixes = SetManager(non_first_name_prefixes)
-        self.suffix_acronyms_ambiguous = SetManager(suffix_acronyms_ambiguous)
+        # untouched defaults (identity check) copy the prebuilt module-level
+        # managers instead of re-validating the raw constants element by
+        # element; user-supplied iterables still get the full check
+        self.prefixes = SetManager(_DEFAULT_PREFIXES if prefixes is PREFIXES else prefixes)
+        self.suffix_acronyms = SetManager(_DEFAULT_SUFFIX_ACRONYMS if suffix_acronyms is SUFFIX_ACRONYMS else suffix_acronyms)
+        self.suffix_not_acronyms = SetManager(_DEFAULT_SUFFIX_NOT_ACRONYMS if suffix_not_acronyms is SUFFIX_NOT_ACRONYMS else suffix_not_acronyms)
+        self.titles = SetManager(_DEFAULT_TITLES if titles is TITLES else titles)
+        self.first_name_titles = SetManager(_DEFAULT_FIRST_NAME_TITLES if first_name_titles is FIRST_NAME_TITLES else first_name_titles)
+        self.conjunctions = SetManager(_DEFAULT_CONJUNCTIONS if conjunctions is CONJUNCTIONS else conjunctions)
+        self.bound_first_names = SetManager(_DEFAULT_BOUND_FIRST_NAMES if bound_first_names is BOUND_FIRST_NAMES else bound_first_names)
+        self.non_first_name_prefixes = SetManager(_DEFAULT_NON_FIRST_NAME_PREFIXES if non_first_name_prefixes is NON_FIRST_NAME_PREFIXES else non_first_name_prefixes)
+        self.suffix_acronyms_ambiguous = SetManager(_DEFAULT_SUFFIX_ACRONYMS_AMBIGUOUS if suffix_acronyms_ambiguous is SUFFIX_ACRONYMS_AMBIGUOUS else suffix_acronyms_ambiguous)
         self.capitalization_exceptions = TupleManager(capitalization_exceptions)
         self.regexes = RegexTupleManager(regexes)
         # Per-bucket delimiter collections that parse_nicknames() consults to