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.

Author: derek73

nameparser/config/__init__.py

@@ -112,9 +112,14 @@ def _normalized_elements(cls, elements: Iterable[str]) -> set[str]:
 
     @classmethod
     def _from_normalized(cls, elements: set[str]) -> 'SetManager':
-        # private fast constructor for sets this class already normalized
-        # (operator results, prebuilt default copies); bypasses __init__
-        # so results aren't re-validated element by element
+        # 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
@@ -241,8 +246,17 @@ def _is_dunder(attr: str) -> bool:
 # 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 real cost on the
-# per-instance-config path, HumanName(constants=None).
+# 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)

tests/test_constants.py

@@ -8,6 +8,7 @@
 from nameparser import HumanName
 from nameparser.config import Constants, RegexTupleManager, SetManager, TupleManager
 from nameparser.config.regexes import EMPTY_REGEX
+from nameparser.config.titles import TITLES
 
 from tests.base import HumanNameTestBase
 
@@ -104,6 +105,15 @@ def test_set_manager_operators_normalize_like_add(self) -> None:
         # pins __rxor__ separately in case it ever stops aliasing __xor__
         self.assertEqual((['Dr.', 'Esq.'] ^ sm).elements, {'mr', 'esq'})
 
+    def test_set_manager_rsub_is_order_sensitive(self) -> None:
+        # __sub__ and __rsub__ are hand-written separately (subtraction
+        # isn't commutative, unlike |/&/^), so a copy-paste operand swap
+        # in __rsub__ would silently flip the result and nothing else
+        # in this file would catch it
+        sm = SetManager(['dr', 'mr'])
+        self.assertEqual((['Dr.', 'Esq.'] - sm).elements, {'esq'})
+        self.assertEqual((sm - ['Dr.', 'Esq.']).elements, {'mr'})
+
     def test_set_manager_constructor_normalizes_like_add(self) -> None:
         # without constructor normalization the operators misfire against
         # the exact spelling visibly stored in the set: & returns empty
@@ -120,6 +130,25 @@ def test_constants_kwarg_elements_are_normalized(self) -> None:
         hn = HumanName("Chemistry Jane Smith", constants=c)
         self.m(hn.title, "Chemistry", hn)
 
+    def test_default_constants_construction_does_not_alias_defaults(self) -> None:
+        # Constants() reuses the prebuilt _DEFAULT_TITLES snapshot via an
+        # identity check instead of re-validating ~1,400 entries; if that
+        # fast path ever returned the shared elements set instead of a
+        # copy, mutating one Constants() instance would corrupt every
+        # other instance's (and the module-level default's) titles
+        c1 = Constants()
+        c1.titles.add('zzz_should_not_leak')
+        c2 = Constants()
+        self.assertNotIn('zzz_should_not_leak', c2.titles)
+        self.assertNotIn('zzz_should_not_leak', TITLES)
+
+    def test_equal_but_not_identical_titles_list_still_validates(self) -> None:
+        # the fast path in Constants.__init__ is an `is` check against the
+        # raw TITLES object, not `==`; an equal-but-copied list must still
+        # go through full normalization rather than accidentally matching
+        c = Constants(titles=list(TITLES) + ['Extra.'])
+        self.assertIn('extra', c.titles)
+
     def test_set_manager_non_str_elements_raise_typeerror(self) -> None:
         # lc() on junk elements either crashes context-free (bytes, int) or
         # silently transmutes None into '' — raise a curated error instead