Skip to content

Commit ef43b83

Browse files
derek73claude
andcommitted
Supply both halves of the emitter's precondition, not one
Review of the previous commit found it half-done, and found the comment block claiming otherwise. The emitter asks two things of two different words. The leading word needs titles & particles; the chained word needs particles_ambiguous. _overlap_parser supplied only the first, so the tests stayed coupled to #360 through 'von'/'van' -- moving those to the never-given half failed seven of them, which is the exact breakage the commit set out to prevent. Constants are now split by role and both halves are supplied. Three claims in that block were also wrong: "drop it from either and there is no chain" -- false for the titles half. Measured, freiherr out of titles: family='von Richthofen', so the chain is intact; what is lost is the REPORT, because the merge sits outside the all(title(x)) guard, and _assign then reports a fork about 'Freiherr' instead. the new test's docstring opened with "both a title and an ambiguous particle" -- verbatim the wrong set the block twenty lines above exists to retire. _overlap_parser's rationale cited 'Do St Johnson', which no test in the file parses; the string lives only in comments. The fixture repeated the docstring's error: it added the leading word to all three sets, so nothing could contradict the wrong precondition. Measured, that third membership is inert -- titles+particles and titles+particles+ambiguous parse byte-identically. Dropped, and control 3 now pins it, which makes the correction executable rather than prose. test_properties.py carried the same borrow on a bigger surface: its 'Freiherr ' lead is one of four in a sweep over every ambiguous particle, and if that lead goes transparent the _group shapes leave the sweep while it keeps passing. Supplied there too. normal 2486 passed leading word out of titles+particles only the cases.py tripwire chained words -> never-given no test in this diff emitter suppressed 8 failed (still sensitive) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 16001c2 commit ef43b83

2 files changed

Lines changed: 88 additions & 41 deletions

File tree

tests/v2/test_parser.py

Lines changed: 78 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -288,18 +288,24 @@ def test_trailing_roman_numeral_reports_the_fork() -> None:
288288
assert parse("John Q. V").ambiguities == ()
289289

290290

291-
#: The words the _group-emitter tests below spell. The emitter asks two
292-
#: DIFFERENT things of two different words, measured 2026-08-14 against
293-
#: "Freiherr von Richthofen":
291+
#: The words the _group-emitter tests below spell, apart from the
292+
#: by-construction test, which invents its own. The emitter asks two
293+
#: DIFFERENT things of two different words. Measured against
294+
#: "Freiherr von Richthofen", one membership dropped at a time:
294295
#:
295-
#: leading word ('Freiherr') must be in titles & PARTICLES. Drop it
296-
#: from either and there is no chain at all -- the particle is the
297-
#: leading name piece again and _assign reports the fork instead.
298-
#: Since #367 a plain title is transparent to the leading-particle
299-
#: exception, which is why a title alone no longer does it.
300-
#: chained word ('von') must be in PARTICLES_AMBIGUOUS. Drop it
301-
#: and the chain still happens (family='von Richthofen') but no
302-
#: fork is reported -- an unambiguous particle is not a decision.
296+
#: leading word ('Freiherr') titles & PARTICLES, and the two halves
297+
#: buy different things. Without `particles` there is no CHAIN --
298+
#: 'von' is the leading name piece again (given='von',
299+
#: family='Richthofen') and _assign reports the fork. Without
300+
#: `titles` the chain still happens (family='von Richthofen') but
301+
#: this emitter's `all(title(x) for x in range(k))` guard fails,
302+
#: so the REPORT is lost and _assign reports a fork about
303+
#: 'Freiherr' instead. The merge sits outside that guard.
304+
#: Its `particles_ambiguous` membership is irrelevant either way,
305+
#: pinned by control 3 in the test below.
306+
#: chained word ('von') PARTICLES_AMBIGUOUS. Drop it and the
307+
#: chain still happens (family='von Richthofen') but no fork is
308+
#: reported -- an unambiguous particle is not a decision.
303309
#:
304310
#: The distinction matters because this file used to assert
305311
#: `titles & particles_ambiguous` for the LEADING word, which is the
@@ -309,49 +315,66 @@ def test_trailing_roman_numeral_reports_the_fork() -> None:
309315
#: subsets of `particles`, so it cannot empty `titles & particles` and
310316
#: cannot orphan this emitter.
311317
#:
312-
#: These tests supply the memberships anyway rather than borrowing them,
313-
#: because reachability is a property of the EMITTER, not of the shipped
314-
#: word lists: a caller may configure the overlap themselves (`Lexicon`
315-
#: asserts no invariant against it and construction warns about nothing).
316-
#: What the SHIPPED vocabulary reaches is a separate claim, pinned where
317-
#: it belongs -- the "Freiherr von Richthofen" row in tests/v2/cases.py,
318-
#: which should fail loudly if #360 ever changes that parse.
318+
#: Both roles are supplied rather than borrowed, because reachability is
319+
#: a property of the EMITTER, not of the shipped word lists: a caller may
320+
#: configure the overlap themselves, and no `Lexicon` invariant forbids
321+
#: it -- constructing one emits no warning. Supplying only the leading
322+
#: half would leave the tests coupled to #360 through the chained word,
323+
#: which is the bug the first cut of this commit shipped.
324+
#:
325+
#: What the SHIPPED vocabulary reaches is a separate claim, pinned in
326+
#: tests/v2/cases.py's "Freiherr von Richthofen" row. Note that row
327+
#: tracks the PARSE, not the memberships: moving `freiherr` between the
328+
#: particle halves leaves it green, and only a change to the leading
329+
#: word's `titles`/`particles` membership, or to `von`'s ambiguous one,
330+
#: moves it.
319331
_TITLE_PARTICLES = frozenset({"freiherr", "do", "st"})
320332

333+
#: The words those tests CHAIN. Disjoint from the leading set on purpose
334+
#: -- the two roles need different memberships, and holding them apart is
335+
#: what keeps that legible.
336+
_CHAINED_PARTICLES = frozenset({"von", "van"})
337+
321338

322339
def _overlap_parser(policy: Policy | None = None) -> Parser:
323-
"""A Parser whose lexicon spells every `_TITLE_PARTICLES` member as a
324-
title, a particle AND an ambiguous particle, whatever the shipped data
325-
says today. All three sets because these words appear in both roles
326-
across the tests below -- 'Do St Johnson' chains `St`, which needs the
327-
ambiguous membership, while `Do` leads and needs the other two."""
340+
"""A Parser whose lexicon gives each word the memberships its ROLE
341+
needs, whatever the shipped data says today: the leading words become
342+
titles and particles, the chained words ambiguous particles.
343+
344+
`particles` covers both because `_SUBSET_FIELDS` requires
345+
`particles_ambiguous <= particles`; the leading words are deliberately
346+
NOT made ambiguous, since that membership does nothing for them and
347+
asserting it is how the wrong set got written down in the first place.
348+
"""
328349
lex = Lexicon.default().add(
329350
titles=_TITLE_PARTICLES,
330-
particles=_TITLE_PARTICLES,
331-
particles_ambiguous=_TITLE_PARTICLES,
351+
particles=_TITLE_PARTICLES | _CHAINED_PARTICLES,
352+
particles_ambiguous=_CHAINED_PARTICLES,
332353
)
333354
return Parser(lexicon=lex, policy=policy or Policy())
334355

335356

336357
def test_the_chained_emitter_is_reachable_by_construction() -> None:
337358
"""_group's PARTICLE_OR_GIVEN emitter fires when a piece that is both a
338-
title and an ambiguous particle sits ahead of the chained particle.
359+
title and a PARTICLE sits ahead of the chained particle.
339360
340361
Asserted against a lexicon built here, so what it pins is the emitter
341-
rather than today's word lists. The control carries the weight: the
342-
SAME input, with the word a plain title instead, takes _assign's
343-
leading-particle branch and reports the other detail -- so a passing
344-
assertion below cannot be the parser doing what it would have done
345-
anyway. The overlap is what routes to _group, not the title.
346-
347-
If this test ever fails, the emitter really is gone or broken. An
348-
empty `titles & particles_ambiguous` in the shipped vocabulary does
349-
NOT fail it, and does not mean the emitter is unreachable.
362+
rather than today's word lists. Three controls carry the weight, one
363+
per membership the claim rests on: drop the leading word's `particles`
364+
and the chain is gone; drop the chained word's `particles_ambiguous`
365+
and the report is gone; drop the leading word's `particles_ambiguous`
366+
and nothing moves at all -- which is the whole correction, since
367+
asserting THAT membership is what this file used to do.
368+
369+
If this test fails, the emitter is gone or broken, or one of the two
370+
words this test builds has lost a membership it supplies itself. An
371+
empty `titles & particles_ambiguous` in the SHIPPED vocabulary does
372+
not fail it and does not mean the emitter is unreachable.
350373
"""
351374
word = "zzoverlap"
352-
base = Lexicon.default()
353-
overlap = base.add(
354-
titles={word}, particles={word}, particles_ambiguous={word})
375+
base = Lexicon.default().add(
376+
particles=_CHAINED_PARTICLES, particles_ambiguous=_CHAINED_PARTICLES)
377+
overlap = base.add(titles={word}, particles={word})
355378
text = f"{word} van Johnson"
356379

357380
# the emitter, reached by construction
@@ -381,6 +404,17 @@ def test_the_chained_emitter_is_reachable_by_construction() -> None:
381404
assert (quiet.given, quiet.family) == (chained.given, chained.family)
382405
assert quiet.ambiguities == ()
383406

407+
# control 3 -- the leading word made ambiguous as well, which is the
408+
# membership the deleted guard test asserted. Byte-identical to the
409+
# treatment, fork included: it buys the emitter nothing. This is the
410+
# executable half of the correction; without it the claim that
411+
# `titles & particles` is the right set lives only in a comment, and
412+
# a fixture supplying all three sets could never contradict it.
413+
also_ambiguous = overlap.add(particles_ambiguous={word})
414+
same = Parser(lexicon=also_ambiguous).parse(text)
415+
assert (same.given, same.family) == (chained.given, chained.family)
416+
assert [a.detail for a in same.ambiguities] == [amb.detail]
417+
384418

385419
def test_ambiguous_particle_reports_both_branches_of_its_fork() -> None:
386420
# "von Richthofen" reads von as a given name and says so. Put a
@@ -509,8 +543,12 @@ def test_chained_particle_detail_is_order_invariant(policy: Policy) -> None:
509543
# "Dr. Van Johnson" is byte-identical to the bare "Van Johnson"
510544
# under every name_order, and the fork comes from _assign -- whose
511545
# detail DOES name the role, unlike the grouping-stage text above.
512-
titled = Parser(policy=policy).parse("Dr. Van Johnson")
513-
bare = Parser(policy=policy).parse("Van Johnson")
546+
# through _overlap_parser as well: 'Van' has to be an ambiguous
547+
# particle for _assign to report anything here, and that membership
548+
# is exactly what #360 may move
549+
p = _overlap_parser(policy)
550+
titled = p.parse("Dr. Van Johnson")
551+
bare = p.parse("Van Johnson")
514552
assert titled.title == "Dr."
515553
assert (titled.given, titled.middle, titled.family) == \
516554
(bare.given, bare.middle, bare.family)

tests/v2/test_properties.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,16 @@ def test_a_leading_ambiguous_particle_is_reported_once_and_only_once(
126126
one is a fork -- so any reconstruction here would have to
127127
re-implement _group rather than check it.
128128
"""
129-
lex = Lexicon.default()
129+
# The 'Freiherr ' lead below has to be a title AND a particle, or it
130+
# is transparent to the leading-particle exception (#367), every
131+
# shape using it leaves _group's emitter for _assign's, and this
132+
# sweep goes on passing with the fork count unchanged -- coverage
133+
# lost silently, which is the failure this sweep is least able to
134+
# notice about itself. Supplied rather than borrowed from the shipped
135+
# vocabulary, for the reason test_parser.py's _TITLE_PARTICLES block
136+
# gives; a no-op against today's data, and #360-proof against
137+
# tomorrow's.
138+
lex = Lexicon.default().add(titles={"freiherr"}, particles={"freiherr"})
130139
# bound-given prefixes are excluded, not overlooked: 'abu' is both
131140
# an ambiguous particle and a bound given prefix, so whether it
132141
# forks depends on whether the bound join fired -- a second rule,

0 commit comments

Comments
 (0)