Skip to content

Commit b30f174

Browse files
committed
Close the review's findings on the gate and the 2.0 ledger
The spaced-honorific rule matched a LEADING 양/군, which are ordinary surnames in that position. Nothing is misclassified today, but a future change routing the leading 양 to suffix would have fallen through to that rule and been reported as an intended honorific -- the regression class the gate exists to catch. Head match dropped; all 89 still classify. UNEXPLAINED kept only the facade's before/after while the diff set unions both surfaces, so an _ambiguities-only diff -- the case _surfaces_for names as the whole reason to compare v2 -- would have printed a name with no field lines under it. Also: pin classify's subset check and V2_FIELDS against Role, both untested and both silent when wrong; drop an unreachable middle from the #298 rule; and correct a claim that this ledger's hand copies are pinned by test_regex_sync, which pins the 1.4 file by name.
1 parent 04d678e commit b30f174

3 files changed

Lines changed: 86 additions & 13 deletions

File tree

tests/v2/test_differential.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,3 +220,26 @@ def test_malformed_rule_error_names_the_ledger_it_came_from() -> None:
220220
compare.validate_rules(bad, "expected_since_2.0.0.toml")
221221
with pytest.raises(SystemExit, match="expected_since_1.4.0.toml"):
222222
compare.validate_rules([{}], "expected_since_1.4.0.toml")
223+
224+
225+
def test_classify_declines_a_diff_touching_a_field_the_rule_omits() -> None:
226+
"""The subset check is the tightness mechanism of every `fields`
227+
rule -- a rule claims a diff only when EVERY changed field is one it
228+
listed. Nothing else pinned it: a rule and a diff that name the same
229+
single field satisfy `<=`, `>=`, `==` and `&` alike, so the existing
230+
tests pass with the comparison flipped, and every deliberate field
231+
omission in both ledgers would quietly stop meaning anything."""
232+
rules = [{"issue": "given-only", "fields": ["given"]}]
233+
assert compare.classify("x", {"given"}, rules) == "given-only"
234+
assert compare.classify("x", {"given", "suffix"}, rules) is None
235+
236+
237+
def test_v2_fields_matches_the_Role_enum() -> None:
238+
"""AGENTS.md: the seven roles are 'defined once and derived
239+
everywhere'. compare.py cannot import Role into the WORKER (that
240+
runs under the old wheel), but this copy reads the working tree's
241+
ParsedName and must track Role. If a role were added and this tuple
242+
not updated, getattr never asks for it and every change in that role
243+
is invisible on the v2 surface -- silent under-coverage, exit 0."""
244+
from nameparser import Role
245+
assert compare.V2_FIELDS == tuple(str(r) for r in Role)

tools/differential/compare.py

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,13 @@
3535
#: The facade's vocabulary is the one that expires, at 3.0.
3636
_V1_TO_ROLE = {"first": "given", "last": "family"}
3737

38+
#: An unclassified diff, carrying BOTH surfaces' before/after:
39+
#: (name, old_facade, new_facade, old_v2, new_v2). Both halves are kept
40+
#: because a diff can exist on the v2 surface alone, and a report that
41+
#: named such a diff without showing it would be unactionable.
42+
_Unexplained = tuple[str, dict[str, str], dict[str, str],
43+
dict[str, object], dict[str, object]]
44+
3845

3946
def _parse_version(text: str) -> tuple[int, int, int]:
4047
"""The numeric release tuple, padded to three parts. Every version
@@ -345,14 +352,21 @@ def main() -> int:
345352
if want_v2:
346353
from nameparser import parse
347354
by_issue: dict[str, list[str]] = {}
348-
unexplained: list[tuple[str, dict[str, str], dict[str, str]]] = []
355+
# BOTH surfaces' old/new are retained, not just the facade's. A diff
356+
# can exist on the v2 surface alone -- an _ambiguities-only change is
357+
# facade-identical by construction, and is the case _surfaces_for
358+
# names as the whole reason to compare v2 -- and keeping only the
359+
# facade dicts would print such a name under UNEXPLAINED with no
360+
# field lines under it at all: a failure nobody can act on.
361+
unexplained: list[_Unexplained] = []
349362
for name, old in zip(corpus, old_rows):
350363
new = {k: v or "" for k, v in HumanName(name).as_dict().items()
351364
if k in FIELDS}
352365
# canonicalized on the way in: the ledger speaks Role's names,
353366
# and the facade is the surface whose vocabulary differs
354367
diff = {_canonical_field(f) for f in FIELDS
355368
if old["facade"].get(f, "") != new.get(f, "")}
369+
new_v2: dict[str, object] = {}
356370
if want_v2:
357371
p = parse(name)
358372
new_v2 = {f: (getattr(p, f, "") or "") for f in V2_FIELDS}
@@ -365,12 +379,13 @@ def main() -> int:
365379
continue
366380
issue = classify(name, diff, rules)
367381
if issue is None:
368-
unexplained.append((name, old["facade"], new))
382+
unexplained.append(
383+
(name, old["facade"], new, old.get("v2", {}), new_v2))
369384
else:
370385
by_issue.setdefault(issue, []).append(name)
371386

372387
changed = [n for names in by_issue.values() for n in names] \
373-
+ [n for n, _, _ in unexplained]
388+
+ [row[0] for row in unexplained]
374389
latin = sum(1 for n in changed if _is_latin_only(n))
375390
print(f"corpus: {len(corpus)} names; "
376391
f"intentional diffs: {sum(map(len, by_issue.values()))}; "
@@ -384,16 +399,25 @@ def main() -> int:
384399
if unexplained:
385400
print("Field names below are Role's, matching what a ledger "
386401
"`fields` rule must say.\n")
387-
for name, old_facade, new in unexplained:
402+
for name, old_facade, new, old_v2, new_v2 in unexplained:
388403
print(f"UNEXPLAINED {name!r}")
404+
# Role's names, not the facade's: this block exists to be turned
405+
# into a ledger rule, and a rule naming the facade's `first`
406+
# would parse, validate, and never match -- with nothing to say
407+
# so. Both surfaces are walked, and a field is reported once
408+
# even when both moved, since one rule entry covers it.
409+
seen: set[str] = set()
389410
for f in FIELDS:
390411
if old_facade.get(f, "") != new.get(f, ""):
391-
# Role's name, not the facade's: this block exists to
392-
# be turned into a ledger rule, and a rule naming the
393-
# facade's `first` would parse, validate, and never
394-
# match -- with nothing to say so.
412+
seen.add(_canonical_field(f))
395413
print(f" {_canonical_field(f)}: "
396414
f"{old_facade.get(f, '')!r} -> {new.get(f, '')!r}")
415+
for f in (*V2_FIELDS, "_ambiguities"):
416+
if old_v2.get(f, "") != new_v2.get(f, "") \
417+
and _canonical_field(f) not in seen:
418+
print(f" {_canonical_field(f)}: "
419+
f"{old_v2.get(f, '')!r} -> {new_v2.get(f, '')!r}"
420+
f" [v2 surface only]")
397421
return 1 if unexplained else 0
398422

399423

tools/differential/expected_since_2.0.0.toml

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,19 @@ issue = "fix(#271/#272/#298) native-script CJK: family-first order, hangul segme
4949
# are absent for the same reason against the compound rules further
5050
# down. The class is copied verbatim from the same rule in
5151
# expected_since_1.4.0.toml rather than rewritten in literal
52-
# characters: tests/v2/test_regex_sync.py pins that spelling against
53-
# the script table, and a hand-rewritten twin drifts silently.
52+
# characters, so the two ledgers spell the same class the same way.
53+
#
54+
# Be aware of what does NOT hold here: tests/v2/test_regex_sync.py pins
55+
# the 1.4 ledger's copy against _policy._SCRIPT_RANGES by hardcoded
56+
# filename, so THIS copy is unpinned. A span removed from the script
57+
# table forces the 1.4 rule to narrow, test-driven, while this twin
58+
# stays wide and starts classifying real regressions as intended. The
59+
# same gap covers the honorific alternations below, hand-copied from
60+
# SUFFIX_NOT_ACRONYMS and GLUED_HONORIFICS (both verified equal to
61+
# their sources 2026-08-05). Parametrizing those pins over every
62+
# expected_since_*.toml is the fix; it needs the #271/#272 selector
63+
# reworked first, since it asserts uniqueness and this file has two
64+
# such rules.
5465
name_regex = "[\\u3005-\\u3006\\u3040-\\u309F\\u30A0-\\u30FF\\u3400-\\u4DBF\\u4E00-\\u9FFF\\uF900-\\uFAFF\\uAC00-\\uD7A3\\uFF65-\\uFF65]"
5566
fields = ["given", "middle", "family", "_ambiguities"]
5667

@@ -115,7 +126,7 @@ issue = "fix(#307/#308/#320) spaced CJK postnominal honorific routed to suffix"
115126
# ('田中 さん', '김민준 박사님') move here too, and #320 lets a trailing
116127
# ASCII period through ('김민준 씨.', '김민준 양.').
117128
#
118-
# Whole-token on the name STRING -- preceded by start, space or comma,
129+
# Whole-token on the name STRING -- preceded by a space or comma,
119130
# followed by end, space or comma, with #320's optional period between.
120131
# That anchor is what the 1.4 ledger's honorific rule exists to
121132
# explain: unanchored, any name ENDING in 양 or 군 would match, and a
@@ -124,11 +135,26 @@ issue = "fix(#307/#308/#320) spaced CJK postnominal honorific routed to suffix"
124135
# SUFFIX_NOT_ACRONYMS, written longest-first so '김민준 박사님' matches
125136
# 박사님 rather than stalling on 박사.
126137
#
138+
# The leading `^` alternative this rule used to carry has been REMOVED,
139+
# because it reopened at the head exactly the hole the tail anchor
140+
# closes. 양 and 군 are honorifics in trailing position and ordinary
141+
# SURNAMES in leading position -- Yang and Gun -- so '양 미선' matched
142+
# on its own surname. Today that name's diff is {given, family} and the
143+
# CJK order rule above claims it first, so nothing was misclassified;
144+
# but a future change routing the leading 양 to `suffix` would give
145+
# {given, family, suffix}, which is no longer a subset of that rule's
146+
# fields, and it would fall through to THIS rule -- whose fields do
147+
# include suffix -- and be reported as an intended honorific. A surname
148+
# silently reclassified as a suffix is the regression class this gate
149+
# exists to catch. A postnominal honorific is by definition not at the
150+
# start of the name, so nothing legitimate needs the head match:
151+
# dropping it leaves all 89 diffs classified (measured 2026-08-05).
152+
#
127153
# `middle` is in `fields` here where the glued rule omits it, and the
128154
# reason is measured: a three-token spaced name loses its middle to the
129155
# order flip while the honorific leaves ('田中 太郎 様' -> given 太郎,
130156
# family 田中, suffix 様; '김민준 박사 씨' likewise).
131-
name_regex = "(?:^|[ ,])(?:박사님|선생님|교수님|박사|씨|님|군|양|先生|女士|小姐|博士|教授|様|氏|殿|さん|さま|くん|ちゃん)\\.?(?=$|[ ,])"
157+
name_regex = "[ ,](?:박사님|선생님|교수님|박사|씨|님|군|양|先生|女士|小姐|博士|教授|様|氏|殿|さん|さま|くん|ちゃん)\\.?(?=$|[ ,])"
132158
fields = ["given", "middle", "family", "suffix"]
133159

134160
[[change]]
@@ -196,4 +222,4 @@ issue = "fix(#298) 间隔号 division changes the comma reading, sending the cre
196222
# different rules in 2.1, and they are indistinguishable enough on
197223
# screen that a literal here would be unreviewable.
198224
name_regex = "(?s)(?=.*,)(?=.*\\u00B7)(?=.*[\\u3005-\\u3006\\u3040-\\u309F\\u30A0-\\u30FF\\u3400-\\u4DBF\\u4E00-\\u9FFF\\uF900-\\uFAFF\\uAC00-\\uD7A3\\uFF65-\\uFF65])"
199-
fields = ["given", "middle", "family", "title", "suffix"]
225+
fields = ["given", "family", "title", "suffix"]

0 commit comments

Comments
 (0)