Actually exercise the surname split under drawn lexicons - #313
Open
derek73 wants to merge 5 commits into
Open
Conversation
The manufactured 'surname + 민준' token was in the pool but rarely in the name: one entry among ~30, against a name of 1-8 pieces. Measured, the lexicon and policy agree on ~14 draws in 250 but the token reached the name only 0-4 times -- and every time it did, the split fired. The shortfall was sampling, not the stage, so insert one at a drawn position when the draw admits it. Twelve randomized runs of 250 now give 0-19 fires (median 8) where they gave 0-2. Also corrects this comment's own claim. It called the surname half 'structurally inert, not luck' on the strength of a single derandomize=True run reporting zero -- but that run is one sample and repeating it cannot disagree with itself. The structural part is real and narrower than stated: a non-hangul surname makes a mixed-script token that can never be a site. The rest was luck.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #313 +/- ##
=======================================
Coverage 98.46% 98.46%
=======================================
Files 41 41
Lines 2803 2803
=======================================
Hits 2760 2760
Misses 43 43 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The note said Han segmentation is "opt-in via locales.ZH, which these policies do not draw", implying the Han entries are inert here. True of locale PACKS, and false of what `_policies` actually generates: it draws `segment_scripts` freely, and HAN lands in 37.7% of drawn policies (measured over 20000 draws). An activated Han token is not scenery either -- standing earlier in the name it takes the surname site and declines, so "欧阳 김민준" does not split where "김민준 欧阳" does. Pre-existing, and two lines from where this branch has been editing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
script_segment's multi-match fork -- two vocabulary entries match the same token, longest-first chooses, and the stage reports the reading it passed over -- was fuzzed by nothing. The cause is structural rather than statistical: the pool's only hangul entries were 김 and 남궁, and no entry was a proper PREFIX of another, so `matches` could never hold more than one length whatever the draw did. 남 is a shipped Korean surname and a prefix of 남궁, so a lexicon drawn with both makes 남궁민준 match twice and sends the parse down the ambiguity-emission-plus-index-remap path. Instrumented rather than assumed, because reachable is not the same as reached: both entries have to land in the same drawn `surnames` (0.8% of draws), so the fork fires about once in 900 examples -- 42 over 36000 measured -- and the committed derandomize=True seed does not reach it. A randomized run is what sees it. Said in the note so the next reader does not conclude from one green run that the entry is doing nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`drawn.insert(...)` writes into the list `st.lists` handed back. Safe today -- that strategy builds a fresh list per draw -- but the hazard class is the kind a fuzzer should not carry: a strategy that ever handed back a shared or memoized list would turn this into a bug in the test harness, reported as a bug in the code under test. The rebuild costs nothing and draws in the same order, so the committed seed still counts 23 surname splits and one peel. Also names the shape honestly in the comment above it: the pool CAN produce that token, it just loses the draw. "cannot deliver on its own" contradicted the paragraph four lines up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Third pass at this comment, and the first two both overstated the same
half from samples too small to carry the claim. What it said and what
measurement says:
* "the conversion from sampled to fired is 100%" -- it is about four in
five (350 of 450 aligned examples over 24 randomized runs of 250).
The conclusion it supported was right and is kept; the number is
gone, replaced by the decline paths, which are all the stage
deciding: a FAMILY_COMMA opts the stage out, or an earlier
script-written token takes the surname site -- a bare drawn surname
('김 김민준'), a hangul post-nominal, or a Han token under a policy
that activated HAN. That last group is what the rewrite deleted and
this restores; a Latin word never takes the site (zero occurrences in
1082 aligned examples), so the note that only Latin blocks it named
the one blocker that does not bite.
* "13-19 [per] 250" alignment, "0-4 times" in the name, "one entry
among ~30", "0-5 per run" for the peel -- measured 5.5% of draws over
20000, a couple per 250, a pool of median size 23, and 0-7 peels per
randomized run. Ranges taken from a handful of runs are dropped or
widened.
* "roughly eight times what it was" -- follows from no measurement I
can reproduce; dropped for the fire counts themselves.
Leads instead with the one figure that cannot rot: instrument _split
under the committed derandomize=True seed and origin/master counts zero
surname splits against two peels, this tree 23 against one. One
command, same answer every time.
Adds what the change does NOT buy, which is the honest headline: this
layer asserts totality and span-exactness only, so every behavioral
mutant in the stage passes here before and after and is killed by
tests/v2/pipeline/test_script_segment.py. The one defect class it can
catch is span arithmetic in _split, and the peel already reached that
path -- writing `base + end` as `base + end + 1`, origin/master finds
it in 8 runs of 10 at a median 1561 shrink calls and 14.6 seconds,
this tree in 10 of 10 at 346 calls and 3.6 seconds.
And two costs that went unmentioned: the insertion drops the stage's
i-is-None early return under a live configuration from 1.3% of
examples to 0.1% (it survives where a drawn quote pair carries the
token into a nickname), and an insertion at the end can suppress a
peel, since the peel site is the last non-post-nominal token.
Correction to this branch's first commit message, which is pushed and
not being amended: it gave the pre-change baseline as "0-2" where the
comment said 0, 1, 1, 2, 2, 4. Neither is the range -- 36 randomized
runs of 250 give 0-9.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tests/v2/test_properties.pymanufactures asurname + "민준"token so the unspaced-surname split inscript_segmentgets exercised under randomly drawn lexicons. It was in the pool but rarely in the name — one entry among ~30, against a name of 1–8 pieces.Instrumented over 250 examples, before:
Conversion from sampled to fired was already 100% — nothing was wrong in the stage, the site scan, or the activation gate. The shortfall was sampling, so this inserts one at a drawn position when the draw admits it (drawn position, not the front: the stage takes the first activated-script token, so both placements are worth covering).
Twelve randomized runs of 250 now give 0, 2, 5, 6, 6, 7, 9, 9, 10, 12, 15, 19 — median 8, against 0–2 before.
Not a guarantee, and the comment says so: the insertion only fires when the drawn lexicon carries a hangul surname and the drawn policy activates hangul, about 14 draws in 250, so a thin run can still reach zero. What changed is that alignment now converts every time instead of one time in five.
Also corrects the comment this replaces
It claimed the surname half was "structurally inert, not luck", citing a single
derandomize=Truerun reporting zero. That run is one sample and repeating it cannot disagree with itself — randomized runs gave 0, 1, 1, 2, 2. The structural part is real but narrower than claimed:w + "민준"on a non-hangul surname is a mixed-script token whoseeffective_scriptisNone, so those candidates can never be a site whatever the policy says. Only a drawn hangul surname makes a usable one. The rest was sampling luck.The comment now also warns to re-measure under
derandomize=Falserather than trusting its numbers, since repeating the committed run returns the same count forever.Scope
The peel half still rides on sampling alone (0–5 per run). It is saturated by case rows and stage tests, so a second forced piece was not worth the distribution shift; the mechanism generalizes if that changes.
Test Plan
uv run --extra ja pytest -q— 2854 passed, 16 skipped, 11 xfaileduv run mypy— clean, 96 filesuv run ruff check nameparser/ tests/— clean_splitand driving the same strategies underderandomize=FalseFollow-up from #311.
🤖 Generated with Claude Code