Apply the simplify pass (#272)

Deduped findings of a four-angle /simplify review, no behavior changes:

* _vocab: extract the shared _classify(normalized) step, so
  effective_script normalizes ONCE instead of twice -- deleting the
  comment block that defended the second normalize.
* _script_segment: the two-character-presumption paragraph was written
  at near-identical length here and in locales/ja.py; the stage keeps
  one sentence and points at the pack that knows namedivider's rules.
* _script_segment: "one split path so nothing drifts" was stated three
  times (module docstring, _pieces, _split); it stays on _split, the
  shared path itself.
* _script_segment: the upper-bound ownership fact was at three sites;
  _split's precondition keeps one clause, the raise site and
  Segmentation's docstring keep the argument.
* tests: six `return None` segmenter callables collapse to the pickle
  test's module-level one plus two named siblings -- two, because the
  override test needs distinct objects for its identity assertion.
* tests: the capture-the-argument closure was written twice; both
  cases now share a _capture() helper beside _fake.
* tests: normalize two files' triple trailing newline to one.
* AGENTS.md: promote the raise-vs-decline segmenter doctrine out of the
  Pickling bullet into its own, leaving Pickling one clause.

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

Author: derek73

AGENTS.md

@@ -131,7 +131,8 @@ The 2.0 rewrite lands as underscore-private modules alongside the v1 code. These
 - **The shim TRANSLATES; it never raises on a config v1 accepted, and never silently changes the parse**: `Constants._snapshot()` is a translation boundary between v1's model and v2's invariants, and every transformation there carries its v1-reachability argument in a comment. Four exist today — `first_name_titles` re-folded per word (v1 joins-then-`lc`, v2 normalizes-then-joins), `suffix_acronyms_ambiguous ∩ acronyms` (a provable no-op), `suffix_words − ambiguous` (v1 already accepts the word via the acronym branch, so the addition is inert there), and `particles_ambiguous ∪ (bound ∩ particles)` (a pinned deviation, `test_bound_never_given_prefix_deviates_on_two_pieces`). When a v1 config cannot satisfy a v2 invariant, work out what v1 actually *does* with it — usually nothing — and reproduce that; weakening the invariant or letting the raise through are both wrong. **Test the case the translation decides**, not one where both branches agree: a test using an input v1 parses identically with and without the config pins nothing.
 - **Reprs are bounded**: render which fields deviate from a named baseline and by how much, never contents (`Lexicon(default + titles: +2)`). `PolicyPatch`'s repr shows only set (non-UNSET) fields; `_order_repr` must never raise even on an unvalidated patch's garbage `name_order` (PolicyPatch defers validation to apply time); the sweep test in `tests/v2/test_reprs.py` pins that no config repr leaks the UNSET sentinel.
 - **Typing/docs**: `from __future__ import annotations`; `frozen=True, slots=True` on every public dataclass; strict-profile mypy flags via per-module overrides in pyproject (`strict = true` itself is not valid per-module). Docstrings state contracts in prose with **no doctest blocks** — `--doctest-modules` makes every example a test; behavior examples go to unit tests per the lean-docs rule. **Document the positive direction of a partial property**: "a non-empty `ambiguities` is a signal to act on" is checkable, while "an empty one means no fork occurred" is a universal negative needing exhaustive verification -- that claim was written twice and falsified twice, at sites the author had not audited.
-- **Pickling**: v2 types must round-trip (`Parser` is picklable by construction, and it holds a `Lexicon`; the one qualifier is the optional `Parser(segmenter=...)` hook — a Parser holding a callable pickles iff that callable does, and the same hook is parse-totality's one exception. Everything inside that exception is a bug in USER CODE, never a fact about the name, so it is surfaced rather than absorbed: the segmenter's own exceptions propagate, and the two protocol violations the stage can detect for itself — an answer of the wrong type, and one cutting at or past the end of the token it was handed — raise `TypeError`/`ValueError` from `_script_segment` for the same reason. The line to hold when adding a check there: a protocol violation by the segmenter's AUTHOR raises, while an adapter's defense against its own third-party library (`locales/ja.py`'s repertoire, length, reconstruction and score guards) declines with `None`, because what those catch is a fact about the content). Every frozen type assigns `_guarded_getstate`/`_guarded_setstate` (`_types.py`) in its class body (`@dataclass(slots=True)` would override inherited pickle methods) — unpickling fails at the LOAD site on field-layout skew, and values are deliberately NOT re-validated (pickle is not a security boundary; canonical state comes from a validated instance). `Lexicon` keeps its own copy of the guard (layering) plus the `mappingproxy` slot rebuild; a new unpicklable slot type needs the same treatment plus a round-trip test.
+- **The segmenter contract**: the optional `Parser(segmenter=...)` hook is parse-totality's ONE exception (locales spec section 4). Everything inside that exception is a bug in USER CODE, never a fact about the name, so it is surfaced rather than absorbed: the segmenter's own exceptions propagate, and the two protocol violations the stage can detect for itself — an answer of the wrong type, and one cutting at or past the end of the token it was handed — raise `TypeError`/`ValueError` from `_script_segment` for the same reason. The line to hold when adding a check there: a protocol violation by the segmenter's AUTHOR raises, while an adapter's defense against its own third-party library (`locales/ja.py`'s repertoire, length, reconstruction and score guards) declines with `None`, because what those catch is a fact about the content.
+- **Pickling**: v2 types must round-trip (`Parser` is picklable by construction, and it holds a `Lexicon`; the one qualifier is that a `Parser` pickles iff its segmenter does — see the segmenter bullet above). Every frozen type assigns `_guarded_getstate`/`_guarded_setstate` (`_types.py`) in its class body (`@dataclass(slots=True)` would override inherited pickle methods) — unpickling fails at the LOAD site on field-layout skew, and values are deliberately NOT re-validated (pickle is not a security boundary; canonical state comes from a validated instance). `Lexicon` keeps its own copy of the guard (layering) plus the `mappingproxy` slot rebuild; a new unpicklable slot type needs the same treatment plus a round-trip test.
 - **One sanctioned global**: the (future) cached default `Parser`. Lazily cached FROZEN singletons (`Lexicon.default()`'s `functools.cache`, the future default parser) are constants, not state; any second piece of module-level MUTABLE state requires amending the conventions doc, on purpose, in review. Sanctioned exceptions, facade layer only: `_config_shim.CONSTANTS` (the v1 shared singleton, mutable by design) and `_facade._WARNED_SUBCLASSES` (the once-per-subclass hook-warning dedup set) — both deleted with the layer in 3.0.
 - **Tests**: all v2 tests live in the `tests/v2/` package (its `conftest.py` overrides the v1 dual-run fixture — v2 code never reads shared `CONSTANTS`), one test module per source module plus the cross-cutting ones (`test_reprs.py`, `test_layering.py`, `test_contracts.py`, `test_properties.py`, `test_benchmark.py`, the `cases.py`/`test_cases.py` table, and `test_regex_sync.py`, which pins every hand-copied pattern or codepoint table against its source wherever the copy lives — including copies outside the package), names stating behavior. Never assert `Lexicon.default()` contents; the narrow sourcing spot-checks in `test_default_sources_v1_vocabulary` that pin the v1→v2 migration contract (e.g. the flipped `particles_ambiguous` model) are the sanctioned exception.
 

nameparser/_pipeline/_script_segment.py

@@ -30,10 +30,9 @@
 packs are corpus ALTERNATIVES, one per corpus; stacking them is for
 genuinely mixed data that accepts that trade. Its Segmentation may
 cut anywhere and any number of times, which is why the split path
-below is written for n cuts and the vocabulary hit is simply its
-one-cut case. One precondition guards it that the vocabulary has no
-twin of: a segmenter answers where an UNDIVIDED name divides, so it is
-consulted only when the gated token is the name part's ONLY
+below takes n cuts. One precondition guards it that the vocabulary
+has no twin of: a segmenter answers where an UNDIVIDED name divides,
+so it is consulted only when the gated token is the name part's ONLY
 script-written one -- "山田 太郎" was divided by its writer and must
 not have its family divided again (a Latin title or suffix draws no
 such boundary). A segmenter's own exceptions PROPAGATE -- the single
@@ -109,14 +108,9 @@
 #: STATISTICALLY divided name carries a SEGMENTATION report, and every
 #: RULE-divided one -- the kana boundary, a two-character name,
 #: namedivider's specific-name rules -- carries none.
-#: One case is worth writing down rather than rediscovering: the
-#: two-character division is a PRESUMPTION, not a measurement (usage.rst
-#: says exactly that), yet namedivider scores it 1.0 and this floor
-#: therefore keeps it silent. That is the divider's own stated
-#: certainty, accepted here as-is rather than second-guessed by a
-#: length rule of ours. If presumption-reporting is ever wanted, the
-#: hook is namedivider's DividedName.algorithm, which names WHICH rule
-#: answered -- not the score, which cannot tell the two apart.
+#: namedivider scores its own two-character rule 1.0, so this floor
+#: keeps that division silent -- see locales/ja.py for why the
+#: presumption is accepted as stated.
 #: Not configurable in 2.x (YAGNI).
 _SEGMENTER_CONFIDENCE_FLOOR = 0.9
 
@@ -136,9 +130,7 @@ def _remap(run: tuple[int, ...], split_at: int,
 
 
 def _pieces(text: str, splits: tuple[int, ...]) -> tuple[str, ...]:
-    """`text` cut at every offset in `splits`: n offsets, n+1 pieces.
-    One idiom for both callers -- the split itself and the report that
-    describes it must never disagree about what the cuts produce."""
+    """`text` cut at every offset in `splits`: n offsets, n+1 pieces."""
     cuts = (0, *splits, len(text))
     return tuple(text[a:b] for a, b in zip(cuts, cuts[1:]))
 
@@ -149,13 +141,11 @@ def _split(state: ParseState, i: int, splits: tuple[int, ...],
     a SEGMENTATION report when there is one.
 
     The offsets arrive non-empty, ascending and interior whatever chose
-    them. From a segmenter: non-empty is the caller's own check, and
+    them. From a segmenter: non-empty is the caller's own check,
     strictly ascending with each >= 1 is Segmentation.__post_init__'s,
-    while the last offset being < len(text) is the caller's bounds
-    check -- that half cannot live in Segmentation, which never sees
-    the text. From the vocabulary: the single offset is >= 1 and
-    < len(text) by the range(cap, 0, -1) construction and its len-1
-    cap.
+    and the last offset is < len(text) -- checked by the caller. From
+    the vocabulary: the single offset is >= 1 and < len(text) by the
+    range(cap, 0, -1) construction and its len-1 cap.
 
     The ONE split path: the vocabulary hit is the single-offset case
     and the segmenter's answer the general one, so neither can drift

nameparser/_pipeline/_vocab.py

@@ -249,6 +249,16 @@ def _normalized_for_script(text: str) -> str | None:
     return unicodedata.normalize("NFC", text)
 
 
+def _classify(normalized: str) -> Script | None:
+    """The FIRST _SCRIPT_PATTERNS entry covering all of `normalized`
+    (already NFC, via _normalized_for_script), else None. Shared by
+    both public classifiers so each of them normalizes exactly once."""
+    for script, pattern in _SCRIPT_PATTERNS.items():
+        if pattern.fullmatch(normalized):
+            return script
+    return None
+
+
 def single_script(text: str) -> Script | None:
     """The one Script whose ranges cover EVERY char of `text`, else
     None (mixed-script text has no well-defined convention to apply;
@@ -259,10 +269,7 @@ def single_script(text: str) -> Script | None:
     normalized = _normalized_for_script(text)
     if normalized is None:
         return None
-    for script, pattern in _SCRIPT_PATTERNS.items():
-        if pattern.fullmatch(normalized):
-            return script
-    return None
+    return _classify(normalized)
 
 
 def effective_script(text: str) -> Script | None:
@@ -275,26 +282,18 @@ def effective_script(text: str) -> Script | None:
     HIRAGANA carrier entry. Pure-katakana stays KATAKANA
     (single_script's answer): a lone katakana token is predominantly a
     transcribed foreign name, so nothing defaults on it."""
-    script = single_script(text)
+    # None for both shapes _JA_PATTERN could never match anyway (empty
+    # text, or all-ASCII text): real work, not a leftover "if text"
+    # guard, since the ASCII case is one a bare emptiness check would
+    # let through. The single normalized copy then serves both the
+    # single-script answer and the license below.
+    normalized = _normalized_for_script(text)
+    if normalized is None:
+        return None
+    script = _classify(normalized)
     if script is not None:
         return script
-    # Renormalizes text that single_script (above) already normalized
-    # once: deliberately NOT hoisted into a shared `_classify(normalized)`
-    # helper. The second call only runs on the fall-through path (a
-    # token single_script could not classify at all, i.e. genuinely
-    # mixed-script or empty/ASCII), never on the common single-script
-    # hit above, so it is a quick re-check on already-NFC text, not
-    # measured work -- the per-char-vs-regex history in this module's
-    # header is what a real cost here would look like, and this isn't
-    # it.
-    # normalized is None for both shapes _JA_PATTERN could never match
-    # anyway (empty text, or the all-ASCII text single_script's fast
-    # path already ruled out) -- real work, not a leftover "if text"
-    # guard: unlike the pre-NFC version, None here also covers the
-    # ASCII case, which single_script's own empty check alone would
-    # not.
-    normalized = _normalized_for_script(text)
-    if normalized is not None and _JA_PATTERN.fullmatch(normalized):
+    if _JA_PATTERN.fullmatch(normalized):
         return Script.HIRAGANA
     return None
 

tests/v2/pipeline/test_script_segment.py

@@ -49,6 +49,19 @@ def segmenter(text: str) -> Segmentation | None:
     return segmenter
 
 
+def _capture(
+        splits: tuple[int, ...] = (2,)) -> tuple[list[str], Segmenter]:
+    """_fake, plus the list of texts it was handed: the two cases below
+    that pin WHETHER and WITH WHAT the stage consults a segmenter need
+    the same closure."""
+    asked: list[str] = []
+
+    def segmenter(text: str) -> Segmentation | None:
+        asked.append(text)
+        return Segmentation(splits)
+    return asked, segmenter
+
+
 def _declines(text: str) -> Segmentation | None:
     return None
 
@@ -232,12 +245,7 @@ def test_vocabulary_wins_over_the_segmenter() -> None:
     # precedence, and the reason parser_for(ZH, JA, segmenter=...)
     # composes: a matched surname is a dictionary certainty, so the
     # segmenter is not even asked
-    asked: list[str] = []
-
-    def seg(text: str) -> Segmentation | None:
-        asked.append(text)
-        return Segmentation((2,))
-
+    asked, seg = _capture()
     state = _run("毛泽东", policy=_JA, segmenter=seg)
     assert _texts(state) == ["毛", "泽东"]
     assert asked == []
@@ -398,14 +406,7 @@ def test_the_segmenter_is_handed_the_gated_token_only() -> None:
     # the difference: it is skipped by the gate (no script) and is not
     # a neighbour for the precondition either, so the consult happens
     # and the argument is observable.
-    asked: list[str] = []
-
-    def seg(text: str) -> Segmentation | None:
-        asked.append(text)
-        return Segmentation((2,))
-
+    asked, seg = _capture()
     out = _run("Dr 山田太郎", policy=_JA, segmenter=seg)
     assert asked == ["山田太郎"]
     assert _texts(out) == ["Dr", "山田", "太郎"]
-
-

tests/v2/test_parser.py

@@ -620,6 +620,17 @@ def _module_level_decline(text: str) -> Segmentation | None:
     return None   # module-level so pickle can find it
 
 
+# Inert sentinels for the plumbing tests below, which only ever compare
+# identity. Two of them, because the override test needs the loser and
+# the winner to be DISTINCT objects or its assertion is vacuous.
+def _decline_a(text: str) -> Segmentation | None:
+    return None
+
+
+def _decline_b(text: str) -> Segmentation | None:
+    return None
+
+
 def test_parser_segmenter_is_keyword_only_and_validated() -> None:
     assert Parser().segmenter is None
     with pytest.raises(TypeError, match="callable"):
@@ -629,11 +640,9 @@ def test_parser_segmenter_is_keyword_only_and_validated() -> None:
 
 
 def test_parser_for_carries_the_base_segmenter() -> None:
-    def seg(text: str) -> Segmentation | None:
-        return None
     # not given: the base's carries through unchanged
-    p = parser_for(locales.get("zh"), base=Parser(segmenter=seg))
-    assert p.segmenter is seg
+    p = parser_for(locales.get("zh"), base=Parser(segmenter=_decline_a))
+    assert p.segmenter is _decline_a
 
 
 def test_parser_for_rejects_a_non_parser_base() -> None:
@@ -642,23 +651,16 @@ def test_parser_for_rejects_a_non_parser_base() -> None:
 
 
 def test_parser_for_takes_a_segmenter_keyword() -> None:
-    def seg(text: str) -> Segmentation | None:
-        return None
-    p = parser_for(locales.get("zh"), segmenter=seg)
-    assert p.segmenter is seg
+    p = parser_for(locales.get("zh"), segmenter=_decline_a)
+    assert p.segmenter is _decline_a
     assert parser_for(locales.get("zh")).segmenter is None
 
 
 def test_parser_for_segmenter_keyword_overrides_the_base() -> None:
     # later wins, the same rule scalar policy fields follow
-    def from_base(text: str) -> Segmentation | None:
-        return None
-
-    def explicit(text: str) -> Segmentation | None:
-        return None
-    p = parser_for(locales.get("zh"), base=Parser(segmenter=from_base),
-                   segmenter=explicit)
-    assert p.segmenter is explicit
+    p = parser_for(locales.get("zh"), base=Parser(segmenter=_decline_a),
+                   segmenter=_decline_b)
+    assert p.segmenter is _decline_b
 
 
 def test_parser_for_segmenter_none_clears_the_base() -> None:
@@ -669,13 +671,11 @@ def test_parser_for_segmenter_none_clears_the_base() -> None:
     # (the test above), and this is how you derive an unsegmented
     # parser from a segmented one without rebuilding its lexicon and
     # policy by hand.
-    def seg(text: str) -> Segmentation | None:
-        return None
-    base = Parser(segmenter=seg)
+    base = Parser(segmenter=_decline_a)
     pack = locales.get("zh")
     assert parser_for(pack, base=base, segmenter=None).segmenter is None
     # ...and the base is untouched: parser_for builds a fresh Parser
-    assert base.segmenter is seg
+    assert base.segmenter is _decline_a
 
 
 def test_parser_picklability_is_conditional_on_the_segmenter() -> None:

tests/v2/test_types.py

@@ -528,5 +528,3 @@ def test_with_field_tokens_rejects_mismatched_roles() -> None:
     with pytest.raises(ValueError, match="has role family, not given"):
         name._with_field_tokens(
             {Role.GIVEN: (Token("x", None, Role.FAMILY),)})
-
-