Skip to content

Commit f5859ca

Browse files
derek73claude
andcommitted
Classify the #271 CJK fixes in the differential harness
Adds the expected_changes.toml rule for the two default changes, scoped to a CJK character class so it can only claim names the behavior can actually reach, and regenerates corpus_issues.jsonl (198 -> 200 names; the generator only ever adds). The rule goes FIRST, which turned out to be load-bearing. classify() takes the first matching rule and fix(suffix-routing) declares no name_regex at all, so with the new rule appended at the end -- where it started -- every CJK diff was claimed by suffix-routing under the wrong label and the new rule was dead on arrival. Caught by running the harness against a synthetic CJK corpus, since neither checked-in corpus contains a single CJK character: build_issues_corpus.py requires an internal space and unspaced names are the shape this classifies. The ordering rule is now written down in both the file header and the README. The character class is a hand copy of _vocab._SCRIPT_RANGES that no import can keep honest, so test_regex_sync.py pins it -- the module's first pin on a file outside the package, and its quietest copy, since the harness runs by hand rather than in CI. Han's astral block is excluded on both sides, deliberately: no corpus name reaches it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent d62c745 commit f5859ca

5 files changed

Lines changed: 93 additions & 1 deletion

File tree

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ Each named attribute (`title`, `first`, etc.) is a `@property` that joins its co
117117
The 2.0 rewrite lands as underscore-private modules alongside the v1 code. These conventions apply to all new-API code and are stricter than the v1 sections above. The full design record (rationale, settled-decision logs, dated amendments) lives in untracked `docs/superpowers/specs/`; this section is the enforceable subset. **A commit that establishes or amends one of these conventions must update this section in the same commit** — grep-driven staleness sweeps miss paraphrased prose (see Workflow above), so write-time maintenance is the mechanism, audits are the backstop.
118118

119119
- **Module layout**: every new module is underscore-private (`_types.py`, `_lexicon.py`, `_policy.py`, `_locale.py`, `_render.py`, `_pipeline/`, `_parser.py`, plus the facade layer: `_facade.py`, `_config_shim.py`). The public import surface is exactly `nameparser` and `nameparser.locales`; `nameparser/__init__.py` holds re-exports and `__all__` only — no logic. Since the M11 swap, the old paths are import-path-preserving re-exports: `nameparser.parser` re-exports the `_facade` `HumanName`, `nameparser.config` re-exports the `_config_shim` names (`Constants`, `CONSTANTS`, `SetManager`, `TupleManager`, `RegexTupleManager`); the `config/` DATA modules stay the vocabulary source through 2.x. The whole facade layer is deleted in 3.0.
120-
- **Facade layer** (`_facade.py`, `_config_shim.py`): the v1-compat `HumanName`/`Constants` over the core. Key mechanisms: `Constants._generation` dirty-tracking (every mutation bumps; facades resolve their `Parser` lazily via `_cached_parser(lexicon, policy)`); `Constants._snapshot()` mirrors `_lexicon._default_lexicon()` (equality-pinned); the facade pickles v1-SHAPED state (component lists, one `__setstate__` path for 1.4 and 2.x blobs; components rebuild via `replace()`, never a re-parse); `_V1_HOOKS` overrides warn once per subclass (#280). The compat contract is the migration spec's promise: warning-free 1.4 code behaves identically except release-log-classified fixes — `tools/differential/` (dev-only, not shipped) verifies this against 1.4-on-PyPI over a 652-name corpus (two files: `corpus.jsonl` from the v1 test banks at a pinned ref, `corpus_issues.jsonl` harvested from the issue tracker; `compare.py` globs `corpus*.jsonl` and fails loudly if none match). `parser.py:NNNN` citations throughout the 2.0 code refer to the PRE-swap v1 file, deleted at the M11 swap; resolve them with `git show 2d5d8c2:nameparser/parser.py`.
120+
- **Facade layer** (`_facade.py`, `_config_shim.py`): the v1-compat `HumanName`/`Constants` over the core. Key mechanisms: `Constants._generation` dirty-tracking (every mutation bumps; facades resolve their `Parser` lazily via `_cached_parser(lexicon, policy)`); `Constants._snapshot()` mirrors `_lexicon._default_lexicon()` (equality-pinned); the facade pickles v1-SHAPED state (component lists, one `__setstate__` path for 1.4 and 2.x blobs; components rebuild via `replace()`, never a re-parse); `_V1_HOOKS` overrides warn once per subclass (#280). The compat contract is the migration spec's promise: warning-free 1.4 code behaves identically except release-log-classified fixes — `tools/differential/` (dev-only, not shipped) verifies this against 1.4-on-PyPI over a checked-in corpus of ~650 names (no exact count here: `corpus_issues.jsonl` grows whenever it is regenerated, and the run prints its own per-file totals) (two files: `corpus.jsonl` from the v1 test banks at a pinned ref, `corpus_issues.jsonl` harvested from the issue tracker; `compare.py` globs `corpus*.jsonl` and fails loudly if none match). `parser.py:NNNN` citations throughout the 2.0 code refer to the PRE-swap v1 file, deleted at the M11 swap; resolve them with `git show 2d5d8c2:nameparser/parser.py`.
121121
- **Layering is enforced by `tests/v2/test_layering.py`** (exact-module matching; `if TYPE_CHECKING:` imports don't count): `_types` imports nothing internal at module level — its rendering delegates import `_render` at call time; `_lexicon` and `_policy` sit above `_types` independently (`_lexicon` may import `nameparser.config.*` DATA modules only — vocabulary is single-sourced from the v1 data modules through 2.x, e.g. `config/maiden_markers.py`); `_locale` sits on `_lexicon`+`_policy` (plus `_types` for the shared pickle mixin); `_render` imports `_types` and `_lexicon` (for `Lexicon.default()` and `_normalize`); `_pipeline/*` imports `_types`+`_lexicon`+`_policy` plus in-package `_pipeline` helpers; `_parser` sits on everything except `_render`; the facade layer (`_facade`, `_config_shim`, `parser`, `config/__init__`, `__main__`) may import anything public plus `_render`; locale pack modules (`locales/*.py`) import `_locale`/`_lexicon`/`_policy` only, and the `locales/__init__` additionally lazy-imports its packs (PEP 562). Extend the test's `ALLOWED` table when adding a module.
122122
- **Canonical field order** — the seven roles in `Role` enum declaration order, defined once and derived everywhere (properties, `as_dict`, reprs, `comparison_key`). Never restate the order literally. `Role` is a `StrEnum`: members compare as their field-name strings, and `tokens_for()` coerces strings via `_coerce_enum`.
123123
- **Method organization**, fixed section order in every class: fields + `__post_init__` validation → alternative constructors → dunders (construction/equality → protocol → operators) → properties → public methods by concern (access → editing → comparison → rendering delegates) → private helpers last, except a helper serving exactly one section may sit at that section's head. Sanctioned deviation, facade layer only: `HumanName` and the shim `Constants` organize by v1 concern groups (`# -- render defaults --`, `# -- config / parsing --`, `# -- fields --`, ..., dunders and pickle last) — the classes mirror v1's own surface and die in 3.0; the canonical order still binds every core type.

tests/v2/test_regex_sync.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,16 @@
88
config/regexes.py changed, the copies would silently diverge with no
99
CI signal. Tests may legally import both sides (test_layering.py's own
1010
convention), so this module is where the promise gets checked.
11+
12+
Layering is the usual reason for a copy but not the only one, so this
13+
module's scope is the PROMISE rather than that one pair of packages:
14+
the comma-set pin below reads _pipeline._state instead of config, and
15+
the last test reaches outside the package altogether, to a TOML file
16+
that could not import a Python constant if it wanted to.
1117
"""
1218
import re
19+
import tomllib
20+
from pathlib import Path
1321

1422
import pytest
1523

@@ -149,3 +157,42 @@ def test_comma_char_matches_the_pipeline_comma_set() -> None:
149157
from nameparser._pipeline._state import COMMA_CHARS
150158

151159
assert set(_render._COMMA_CHAR.pattern.strip("[]")) == set(COMMA_CHARS)
160+
161+
162+
def test_differential_cjk_rule_matches_the_script_ranges() -> None:
163+
"""The #271 rule in tools/differential/expected_changes.toml hand-
164+
copies the CJK spans from _vocab._SCRIPT_RANGES into a character
165+
class. A TOML file cannot import the constant, so this is the one
166+
copy with no possible alternative -- and the one whose divergence
167+
is quietest, because the harness is run by hand rather than in CI.
168+
169+
Both failure directions matter, which is why this compares sets
170+
rather than checking coverage. A span MISSING from the class turns
171+
an intended #271 change into an UNEXPLAINED diff (a release
172+
blocker for the wrong reason); a span that should not be there
173+
silently classifies a real regression as intended, which is the
174+
failure the whole harness exists to prevent.
175+
176+
Han's astral block is out of scope on both sides. The rule omits
177+
it deliberately -- no corpus name reaches it, see the comment
178+
there -- so the comparison runs over the BMP spans only, and a new
179+
BMP script added to _SCRIPT_RANGES still fails here until the rule
180+
covers it.
181+
"""
182+
toml_path = (Path(__file__).parents[2] / "tools" / "differential"
183+
/ "expected_changes.toml")
184+
rules = tomllib.loads(toml_path.read_text())["change"]
185+
matched = [r for r in rules if "#271" in r["issue"]]
186+
assert len(matched) == 1, (
187+
f"expected exactly one #271 rule in {toml_path.name}, "
188+
f"found {len(matched)}")
189+
declared = {
190+
(int(lo, 16), int(hi, 16))
191+
for lo, hi in re.findall(r"\\u([0-9A-Fa-f]{4})-\\u([0-9A-Fa-f]{4})",
192+
matched[0]["name_regex"])}
193+
expected = {span
194+
for spans in _vocab._SCRIPT_RANGES.values()
195+
for span in spans if span[1] <= 0xFFFF}
196+
assert declared == expected, (
197+
f"{toml_path.name}'s #271 name_regex declares {sorted(declared)}; "
198+
f"_SCRIPT_RANGES' BMP spans are {sorted(expected)}")

tools/differential/README.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,15 @@ matches only if the observed diff fields are a subset of this list).
116116
Keep both as tight as the actual diff allows -- a loose rule can mask
117117
a real regression.
118118

119+
Rules are tried in file order and the first match wins, so a rule
120+
narrowed by `name_regex` has to sit **above** any rule narrowed only
121+
by `fields`. The `fields`-only rules are broad by construction: a
122+
`fields = ["first", "last", "suffix"]` rule claims every diff confined
123+
to those three columns, whatever the name looked like. Adding the
124+
`fix(#271)` CJK rule at the end of the file was enough to make it dead
125+
on arrival -- `fix(suffix-routing)` claimed all of its diffs first, and
126+
labelled them as something else.
127+
119128
Some entries in the seed list are for behavior families that this
120129
particular corpus (pre-M12 v1 test strings) happens not to contain any
121130
example of (e.g. custom suffix-delimiter rendering, which only fires

tools/differential/corpus_issues.jsonl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@
102102
"John Smith MA"
103103
"John Smith Ma"
104104
"John Smith, Dr."
105+
"John Smith, LEED AP"
105106
"John Smith, Ph. D."
106107
"John Smith, Ph.D."
107108
"John V"
@@ -166,6 +167,7 @@
166167
"Smith, John"
167168
"Smith, John ,"
168169
"Smith, John E, III, Jr"
170+
"Smith, LEED AP"
169171
"St. ___"
170172
"Steven Hardman"
171173
"Steven Hardman, MD - DO - DDS"

tools/differential/expected_changes.toml

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,40 @@
33
# S5). Rules are seeded from docs/superpowers/plans/notes-m12-diffs.md
44
# and the `classification="fix(...)"` rows in tests/v2/cases.py; keep
55
# each entry's `name_regex`/`fields` as tight as the diff allows.
6+
# ORDER MATTERS: classify() takes the FIRST matching rule, so a rule
7+
# narrowed by `name_regex` must sit ahead of any rule narrowed only by
8+
# `fields`, or the broad one silently claims its diffs under the wrong
9+
# label. See the #271 entry immediately below for a case that did.
10+
11+
[[change]]
12+
issue = "fix(#271) native-script CJK: family-first order + hangul segmentation"
13+
# '毛 泽东', '김민준': script_orders flips first/last for a name written
14+
# wholly in Han or Hangul, and the Korean surnames that now ship as
15+
# default vocabulary additionally split an unspaced hangul token into
16+
# last + first. Seeded from the fix(#271) rows in tests/v2/cases.py.
17+
#
18+
# Scoped by script, which is exactly the scope of the behavior --
19+
# neither change can touch a name with no CJK character in it. Han's
20+
# astral block (U+20000-U+323AF) is deliberately left out of the class:
21+
# no name in either corpus reaches it, and a rule should be no wider
22+
# than the diffs it has to explain. Extend it -- and the sync pin in
23+
# tests/v2/test_regex_sync.py, which fails if the two disagree -- if
24+
# one ever does.
25+
#
26+
# FIRST on purpose, and this position is load-bearing. classify()
27+
# returns the first matching rule, and 'fix(suffix-routing)' below
28+
# declares no name_regex at all -- so with this rule anywhere after it,
29+
# every CJK diff was classified as a suffix-routing change instead
30+
# (verified: all four names of a synthetic CJK corpus landed there).
31+
# The label would have been wrong and this rule dead. A rule narrowed
32+
# by name_regex belongs ahead of every rule narrowed only by `fields`.
33+
#
34+
# Expected to match nothing against the current corpora, which contain
35+
# no CJK at all: build_issues_corpus.py requires an internal space, and
36+
# unspaced names are the shape this classifies. Kept for the reason the
37+
# suffix-delimiter rule below is kept -- ready the moment a name lands.
38+
name_regex = "[\\u3400-\\u4DBF\\u4E00-\\u9FFF\\uF900-\\uFAFF\\uAC00-\\uD7A3]"
39+
fields = ["first", "middle", "last"]
640

741
[[change]]
842
issue = "fix(#274) maiden markers consumed"

0 commit comments

Comments
 (0)