Apply the five-agent PR review round (#272)

Two behavior changes, drawing one line the segmenter path did not
previously state: a protocol violation by the segmenter's AUTHOR
raises, while an adapter's defense against its third-party library
declines.

A segmenter answer cutting at or past the end of the token it was
handed now raises ValueError naming the offset and the length,
matching the wrong-answer-type TypeError beside it. Both are
stage-detectable bugs in user code, inside the already-declared
totality exception; the old silent decline made an off-by-one
segmenter undebuggable, since every answer it gave simply vanished
and the name merely looked undivided.

The ja adapter's confidence clamp becomes epsilon-tolerant-else-
decline. Float noise at the edge of namedivider's softmax still
clamps into [0, 1] -- sized for float32, whose eps is ~1.2e-7, since
rule-based answers score exactly 1.0 and a tighter band would reject
a correct division that came back one ulp high. A score outside that
band does not clamp: a divider reporting 87.0 is broken and its
division is worth no more than its score, so the adapter declines,
which keeps the parse safe and observable by the report's absence
where the old clamp mapped garbage to "certain" and silenced it. NaN
declines through the same comparison.

Also: parser_for's segmenter takes the UNSET sentinel, so an explicit
None clears the base's rather than reading as "not given"; the
SEGMENTATION docstring covers both of its sources; a bare-string
guard on Segmentation.splits; the U+3005 rationale corrected in four
files (it is Script=Han, and the BLOCK table is what needed the
singleton); the composition claim qualified wherever it appears; and
tests for the lot, including the range-disjointness pin, the
capture-the-argument and unactivated-neighbour stage cases, and the
lone-hiragana, astral-kanji and ZH+JA-collision integrations.

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

Author: derek73

.github/workflows/python-package.yml

@@ -89,4 +89,12 @@ jobs:
     - name: Install dependencies
       run: uv sync --group dev --extra ja
     - name: Run Tests
-      run: uv run pytest tests/v2/ -q
+      # The import check first: every test the extra adds is guarded by
+      # a find_spec("namedivider") skipif, so a job where the extra
+      # silently failed to install is a job where all of them SKIP and
+      # the step still passes green. This line is what makes that
+      # failure loud -- without it the job proves nothing on the day it
+      # matters.
+      run: |
+        uv run python -c "import namedivider"
+        uv run pytest tests/v2/ -q

AGENTS.md

@@ -131,7 +131,7 @@ 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: a user-supplied segmenter's own exceptions propagate rather than being swallowed as content errors). 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.
+- **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.
 - **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.
 

docs/release_log.rst

@@ -8,7 +8,7 @@ Release Log
     - Add ``Lexicon.surnames``, ``Policy.script_orders``, ``Policy.segment_scripts``, the ``Script`` enum and the ``DEFAULT_SCRIPT_ORDERS`` constant to the public API. This is the first behavior nameparser keys on the script a name is written in; it is allowed only where the script itself settles a convention, never as a proxy for guessing the language (#271)
     - Add ``AmbiguityKind.SEGMENTATION``, reported when a surname split had a vocabulary-supported alternative: ``"남궁민수"`` is 남궁 + 민수 by the compound surname but 남 + 궁민수 by the single-syllable one, and longest-match had to pick. A name with only one possible split decided nothing and reports nothing (#271)
     - Add the Japanese locale pack ``locales.JA`` and the segmenter factory ``locales.ja_segmenter()``, which together divide an unspaced Japanese name: ``parser_for(locales.JA, segmenter=locales.ja_segmenter())`` reads ``山田太郎`` as family ``山田``, given ``太郎``. The two halves are separate because no surname list can do this job — family and given names draw on the same kanji and the reading, not the spelling, decides most divisions — so the pack activates the stage and a third-party divider performs it. ``ja_segmenter()`` wraps `namedivider-python <https://pypi.org/project/namedivider-python/>`_, installed with the new ``nameparser[ja]`` extra; the core stays dependency-free, and ``ja_segmenter(gbdt=True)`` selects namedivider's more accurate gradient-boosted model, which downloads its data on first use. With both packs registered, ``locales.available()`` is now ``('ja', 'ru', 'tr_az', 'zh')`` (closes #272)
-    - Add ``Segmentation`` and the ``Segmenter`` type alias to the public API, plus the keyword-only ``Parser(segmenter=...)`` hook they describe: any callable from a token's text to a ``Segmentation`` (the interior offsets to cut at, and a confidence) or ``None`` to decline. It is consulted only for scripts listed in ``Policy.segment_scripts``, and only where the surname vocabulary declined first, so ``parser_for(locales.ZH, locales.JA, segmenter=...)`` composes — a listed Chinese surname wins, the segmenter takes the rest (#272)
+    - Add ``Segmentation`` and the ``Segmenter`` type alias to the public API, plus the keyword-only ``Parser(segmenter=...)`` hook they describe: any callable from a token's text to a ``Segmentation`` (the interior offsets to cut at, and a confidence) or ``None`` to decline. It is consulted only for scripts listed in ``Policy.segment_scripts``, and only where the surname vocabulary declined first, so ``parser_for(locales.ZH, locales.JA, segmenter=...)`` composes — a listed Chinese surname wins, the segmenter takes the rest. Read that composition with the zh pack's own warning still attached: vocabulary-first means a Japanese kanji name opening on a listed Chinese surname never reaches the segmenter, so ``高橋一郎`` still splits ``高`` + ``橋一郎`` under the stack exactly as it does under ``locales.ZH`` alone. The two packs are alternatives, one per corpus; stack them only for genuinely mixed data that accepts that trade (#272)
     - Add the ``Script`` members ``HIRAGANA`` and ``KATAKANA``. Two members rather than one ``KANA`` because the parser treats them differently: hiragana never transcribes a foreign name, while a wholly-katakana name usually is one (#272)
 
     **Breaking Changes**

docs/usage.rst

@@ -296,11 +296,19 @@ the presumption Japanese practice makes; that is an accepted
 presumption, not a measurement, so a two-character token is the shape
 to check first if a division looks wrong.
 
-A segmenter that answers outside the token it was given — a cut past
-the end of the text, or pieces that do not reconstruct the input — is
-declined silently and the token is left whole. Exceptions are the one
-thing that does *not* stay inside the parse: a segmenter is your code,
-so its errors propagate rather than being absorbed as content errors.
+A segmenter that answers outside the token it was given — a cut at or
+past the end of the text — has violated the protocol, and the parse
+says so rather than hiding it: it raises ``ValueError`` naming the
+offending offset and the token's length, the way an answer of the
+wrong type raises ``TypeError``. Declining silently would leave an
+off-by-one segmenter invisible; every answer it gave would vanish and
+the name would merely look undivided. The shipped ``ja_segmenter()``
+does decline — returns ``None``, token left whole — for the cases that
+are *not* protocol violations: text outside the Japanese repertoire,
+text too short to divide, an answer that fails to reconstruct its
+input, and a score outside [0, 1]. Exceptions are the one thing that
+does *not* stay inside the parse: a segmenter is your code, so its
+errors propagate rather than being absorbed as content errors.
 
 The command line takes the pack but not the segmenter: ``python -m
 nameparser --locale ja`` has no way to attach one, so it activates

nameparser/_parser.py

@@ -18,7 +18,7 @@
 from nameparser._pipeline import run
 from nameparser._pipeline._assemble import assemble
 from nameparser._pipeline._state import ParseState
-from nameparser._policy import UNSET, Policy, PolicyPatch, apply_patch
+from nameparser._policy import UNSET, Policy, PolicyPatch, _Unset, apply_patch
 from nameparser._types import (
     FOLDED_TAG, ParsedName, Segmenter, Token, _guarded_getstate,
     _guarded_setstate, _validated_field_strings,
@@ -187,7 +187,7 @@ def parse(text: str) -> ParsedName:
 
 
 def parser_for(*locales: Locale, base: Parser | None = None,
-               segmenter: Segmenter | None = None) -> Parser:
+               segmenter: Segmenter | None | _Unset = UNSET) -> Parser:
     """Lexicon fragments unioned left-to-right onto base's; policy
     patches applied left-to-right (later wins; set-valued fields union
     per the patch metadata). Validation errors raised while applying a
@@ -199,23 +199,30 @@ def parser_for(*locales: Locale, base: Parser | None = None,
     A ``segmenter`` is passed straight through to the built Parser --
     ``parser_for(locales.JA, segmenter=locales.ja_segmenter())`` is how
     a pack and a segmenter combine, since packs are pure data and
-    cannot supply one. Given explicitly it OVERRIDES base's (later
-    wins, the rule scalar policy fields follow); omitted, base's
-    carries through unchanged."""
+    cannot supply one. The argument has THREE states, the same
+    :data:`~nameparser.UNSET` spelling a PolicyPatch field uses, because
+    None is a meaningful value here and not an absence: omitted (UNSET)
+    carries base's segmenter through unchanged; a callable OVERRIDES
+    base's (later wins, the rule scalar policy fields follow); and an
+    explicit ``None`` CLEARS base's, which is how you derive an
+    unsegmented parser from a segmented one without rebuilding its
+    lexicon and policy by hand."""
     if base is not None and not isinstance(base, Parser):
         raise TypeError(f"base must be a Parser or None, got {base!r}")
     for loc in locales:
         if not isinstance(loc, Locale):
             raise TypeError(f"parser_for() takes Locale packs, got {loc!r}")
     lexicon = base.lexicon if base is not None else Lexicon.default()
     policy = base.policy if base is not None else Policy()
-    # Unpacked here with its siblings because the return builds a FRESH
-    # Parser: any field not listed there silently takes its default, and
-    # a dropped segmenter would be invisible. None reads as "not given"
-    # rather than "clear it" (the escape hatch is a plain Parser:
-    # Parser(lexicon=p.lexicon, policy=p.policy)).
-    if segmenter is None and base is not None:
-        segmenter = base.segmenter
+    # Resolved here rather than at the return because the return builds
+    # a FRESH Parser: any field not listed there silently takes its
+    # default, and a dropped segmenter would be invisible. UNSET, not
+    # None, is what "not given" means -- None is the CLEAR request, and
+    # collapsing the two would make an explicit
+    # parser_for(..., segmenter=None) silently inherit the very
+    # segmenter it was asked to drop.
+    if segmenter is UNSET:
+        segmenter = base.segmenter if base is not None else None
     scalar_setters: dict[str, str] = {}
     for loc in locales:
         for f in dataclasses.fields(PolicyPatch):