From 16001c2f910d750c8238a5d10a9724395faa237e Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Thu, 13 Aug 2026 22:36:43 -0700 Subject: [PATCH 1/2] Pin the chained emitter to the emitter, not to the word lists The _group PARTICLE_OR_GIVEN tests borrowed their precondition from the shipped vocabulary, which coupled them to #360's curation. Two things turned out to be wrong with that, both measured against "Freiherr von Richthofen": The guard asserted the wrong set. It required a lead word in `titles & particles_ambiguous`; the emitter requires `titles & PARTICLES`: freiherr out of particles_ambiguous only -> still chains freiherr out of particles entirely -> no chain freiherr out of titles only -> no chain The two intersections are the same three words today, which is why it read as correct. And #360 cannot empty the right one: it moves words between the may-be-given and never-given halves, both subsets of particles, so particle membership survives either way. The coupling was unnecessary regardless. Reachability is a property of the emitter -- a caller may configure a titles/particles overlap themselves, with no invariant against it and no warning -- so an empty shipped intersection would leave the emitter reachable and merely unexercised by default. The old guard's advice to "remove the emitter rather than repointing them" would have deleted live code. Tests now supply the memberships. The new reachability test carries two controls, one per role: a plain-title lead (no chain, _assign reports instead) and an unambiguous chained word (chain fires, nothing to report) -- so a pass cannot be the grouping it would have produced anyway. Verified: normal 91 passed shipped overlap emptied 91 passed (decoupled) emitter suppressed 6 failed (still sensitive) Co-Authored-By: Claude Opus 5 --- tests/v2/test_parser.py | 139 +++++++++++++++++++++++++++++----------- 1 file changed, 102 insertions(+), 37 deletions(-) diff --git a/tests/v2/test_parser.py b/tests/v2/test_parser.py index a2ae0e28..58b0839a 100644 --- a/tests/v2/test_parser.py +++ b/tests/v2/test_parser.py @@ -288,37 +288,98 @@ def test_trailing_roman_numeral_reports_the_fork() -> None: assert parse("John Q. V").ambiguities == () -#: The word the _group-emitter tests below lead with. It has to be BOTH a -#: title and an ambiguous particle -- see test_the_chained_emitter_is_still -#: _reachable for why, and for what to do when this stops being true. -_TITLE_PARTICLE = "Freiherr" - - -def test_the_chained_emitter_is_still_reachable() -> None: - """_group's PARTICLE_OR_GIVEN emitter needs a piece that is both a title - and an ambiguous particle somewhere ahead of the chained particle, and - since #367 nothing else will do -- an ordinary title is transparent to - the leading-particle exception, so on its own it takes _assign's branch - instead. That word need not lead the input: "Dr. Do van Johnson" reaches - the emitter with a plain title in front of it. What it cannot be is - absent. - - Two different failures, wanting two different fixes. If the intersection - is merely missing the word the tests below use, pick another from the - set. If it is EMPTY, the emitter is unreachable: every test of it is - then measuring nothing, and the emitter itself should go rather than be - re-pointed. #360 may move members of this set, which is why the coupling - is executable here instead of being a comment. +#: The words the _group-emitter tests below spell. The emitter asks two +#: DIFFERENT things of two different words, measured 2026-08-14 against +#: "Freiherr von Richthofen": +#: +#: leading word ('Freiherr') must be in titles & PARTICLES. Drop it +#: from either and there is no chain at all -- the particle is the +#: leading name piece again and _assign reports the fork instead. +#: Since #367 a plain title is transparent to the leading-particle +#: exception, which is why a title alone no longer does it. +#: chained word ('von') must be in PARTICLES_AMBIGUOUS. Drop it +#: and the chain still happens (family='von Richthofen') but no +#: fork is reported -- an unambiguous particle is not a decision. +#: +#: The distinction matters because this file used to assert +#: `titles & particles_ambiguous` for the LEADING word, which is the +#: wrong set. It reads as correct only because the two intersections are +#: the same three words today. #360 moves words between the may-be-given +#: and never-given halves of the particle vocabulary; both halves are +#: subsets of `particles`, so it cannot empty `titles & particles` and +#: cannot orphan this emitter. +#: +#: These tests supply the memberships anyway rather than borrowing them, +#: because reachability is a property of the EMITTER, not of the shipped +#: word lists: a caller may configure the overlap themselves (`Lexicon` +#: asserts no invariant against it and construction warns about nothing). +#: What the SHIPPED vocabulary reaches is a separate claim, pinned where +#: it belongs -- the "Freiherr von Richthofen" row in tests/v2/cases.py, +#: which should fail loudly if #360 ever changes that parse. +_TITLE_PARTICLES = frozenset({"freiherr", "do", "st"}) + + +def _overlap_parser(policy: Policy | None = None) -> Parser: + """A Parser whose lexicon spells every `_TITLE_PARTICLES` member as a + title, a particle AND an ambiguous particle, whatever the shipped data + says today. All three sets because these words appear in both roles + across the tests below -- 'Do St Johnson' chains `St`, which needs the + ambiguous membership, while `Do` leads and needs the other two.""" + lex = Lexicon.default().add( + titles=_TITLE_PARTICLES, + particles=_TITLE_PARTICLES, + particles_ambiguous=_TITLE_PARTICLES, + ) + return Parser(lexicon=lex, policy=policy or Policy()) + + +def test_the_chained_emitter_is_reachable_by_construction() -> None: + """_group's PARTICLE_OR_GIVEN emitter fires when a piece that is both a + title and an ambiguous particle sits ahead of the chained particle. + + Asserted against a lexicon built here, so what it pins is the emitter + rather than today's word lists. The control carries the weight: the + SAME input, with the word a plain title instead, takes _assign's + leading-particle branch and reports the other detail -- so a passing + assertion below cannot be the parser doing what it would have done + anyway. The overlap is what routes to _group, not the title. + + If this test ever fails, the emitter really is gone or broken. An + empty `titles & particles_ambiguous` in the shipped vocabulary does + NOT fail it, and does not mean the emitter is unreachable. """ - lex = Lexicon.default() - both = lex.titles & lex.particles_ambiguous - assert both, ( - "no title is an ambiguous particle, so _group's PARTICLE_OR_GIVEN " - "emitter is unreachable and its tests measure nothing -- remove the " - "emitter rather than repointing them") - assert _TITLE_PARTICLE.lower() in both, ( - f"{_TITLE_PARTICLE!r} is no longer both a title and an ambiguous " - f"particle; the _group-emitter tests need a lead from {sorted(both)}") + word = "zzoverlap" + base = Lexicon.default() + overlap = base.add( + titles={word}, particles={word}, particles_ambiguous={word}) + text = f"{word} van Johnson" + + # the emitter, reached by construction + chained = Parser(lexicon=overlap).parse(text) + assert (chained.given, chained.family) == ("", "van Johnson") + (amb,) = chained.ambiguities + assert amb.kind is AmbiguityKind.PARTICLE_OR_GIVEN + assert amb.detail == ( + "'van' was chained onto the following name piece; " + "it is also a given name in other names") + + # control 1 -- leading word a plain title, NOT a particle: no chain + # at all, and _assign reports the other side of the same fork. + title_only = Parser(lexicon=base.add(titles={word})).parse(text) + assert (title_only.given, title_only.family) == ("van", "Johnson") + assert [a.detail for a in title_only.ambiguities] == [ + "leading 'van' may be a family-name particle; read as a given name"] + + # control 2 -- overlap intact but the CHAINED word unambiguous: the + # chain still fires, so the parse matches, and the only difference is + # that there is no decision left to report. Without this control the + # assertion above could not tell "the emitter ran" from "the chain + # happened to produce this grouping". + unambiguous = dataclasses.replace( + overlap, particles_ambiguous=overlap.particles_ambiguous - {"van"}) + quiet = Parser(lexicon=unambiguous).parse(text) + assert (quiet.given, quiet.family) == (chained.given, chained.family) + assert quiet.ambiguities == () def test_ambiguous_particle_reports_both_branches_of_its_fork() -> None: @@ -338,19 +399,23 @@ def test_ambiguous_particle_reports_both_branches_of_its_fork() -> None: # still reaches _group's emitter. This is the canonical spelling of # that, not the only one: "St Van Johnson", "Do St Johnson" and # "Dr. Do van Johnson" reach it as well. - given_reading = parse("von Richthofen") + # + # Parsed through _overlap_parser so the memberships are supplied + # rather than borrowed -- see _TITLE_PARTICLES. + p = _overlap_parser() + given_reading = p.parse("von Richthofen") assert given_reading.given == "von" assert [a.kind for a in given_reading.ambiguities] == \ [AmbiguityKind.PARTICLE_OR_GIVEN] - particle_reading = parse("Freiherr von Richthofen") + particle_reading = p.parse("Freiherr von Richthofen") assert particle_reading.family == "von Richthofen" assert [a.kind for a in particle_reading.ambiguities] == \ [AmbiguityKind.PARTICLE_OR_GIVEN] assert [t.text for t in particle_reading.ambiguities[0].tokens] == ["von"] # and the branch the title no longer takes: "Dr. Van Johnson" is # now byte-identical to the bare "Van Johnson", fork included - titled, bare = parse("Dr. Van Johnson"), parse("Van Johnson") + titled, bare = p.parse("Dr. Van Johnson"), p.parse("Van Johnson") assert (titled.given, titled.family) == (bare.given, bare.family) \ == ("Van", "Johnson") assert [a.detail for a in titled.ambiguities] == \ @@ -391,7 +456,7 @@ def test_bound_given_name_that_is_also_a_particle() -> None: "Dr. Van Jr.", "Dr. Van MD", "Dr. Do Jr.", ]) def test_no_op_prefix_chain_is_not_a_fork(text: str) -> None: - assert parse(text).ambiguities == () + assert _overlap_parser().parse(text).ambiguities == () def test_a_fork_is_reported_by_exactly_one_stage() -> None: @@ -400,7 +465,7 @@ def test_a_fork_is_reported_by_exactly_one_stage() -> None: # 'Do' rather than the 'Dr.' this used until 2.2, for the reason # above: with a plain title the chain loop never fires at all now, # so the double-report it guards against is out of reach there. - n = parse("Do Van Jr Smith") + n = _overlap_parser().parse("Do Van Jr Smith") assert n.given == "Van" assert len(n.ambiguities) == 1 @@ -411,7 +476,7 @@ def test_chained_particle_detail_does_not_claim_a_role() -> None: # puts it in GIVEN, while the bare "Freiherr von Richthofen" above # puts it in FAMILY. The detail must describe the decision, not # guess a role. - n = parse("Freiherr von Richthofen de la Cruz") + n = _overlap_parser().parse("Freiherr von Richthofen de la Cruz") assert n.given == "von Richthofen" (amb,) = n.ambiguities assert "family name" not in amb.detail @@ -431,7 +496,7 @@ def test_chained_particle_detail_is_order_invariant(policy: Policy) -> None: # same one three times -- pin it, or the invariant is only an # intention. ("Dr. Van Johnson" carried this until 2.2; #367 made a # plain title transparent, so it no longer chains at all.) - n = Parser(policy=policy).parse("Freiherr von Richthofen") + n = _overlap_parser(policy).parse("Freiherr von Richthofen") assert (n.given, n.family) == ("", "von Richthofen") (amb,) = n.ambiguities assert amb.kind is AmbiguityKind.PARTICLE_OR_GIVEN From ef43b83c87828dcd5d571efa9445c7f48f82fffd Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Fri, 14 Aug 2026 23:28:18 -0700 Subject: [PATCH 2/2] 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 --- tests/v2/test_parser.py | 118 ++++++++++++++++++++++++------------ tests/v2/test_properties.py | 11 +++- 2 files changed, 88 insertions(+), 41 deletions(-) diff --git a/tests/v2/test_parser.py b/tests/v2/test_parser.py index 58b0839a..cc1f9d30 100644 --- a/tests/v2/test_parser.py +++ b/tests/v2/test_parser.py @@ -288,18 +288,24 @@ def test_trailing_roman_numeral_reports_the_fork() -> None: assert parse("John Q. V").ambiguities == () -#: The words the _group-emitter tests below spell. The emitter asks two -#: DIFFERENT things of two different words, measured 2026-08-14 against -#: "Freiherr von Richthofen": +#: The words the _group-emitter tests below spell, apart from the +#: by-construction test, which invents its own. The emitter asks two +#: DIFFERENT things of two different words. Measured against +#: "Freiherr von Richthofen", one membership dropped at a time: #: -#: leading word ('Freiherr') must be in titles & PARTICLES. Drop it -#: from either and there is no chain at all -- the particle is the -#: leading name piece again and _assign reports the fork instead. -#: Since #367 a plain title is transparent to the leading-particle -#: exception, which is why a title alone no longer does it. -#: chained word ('von') must be in PARTICLES_AMBIGUOUS. Drop it -#: and the chain still happens (family='von Richthofen') but no -#: fork is reported -- an unambiguous particle is not a decision. +#: leading word ('Freiherr') titles & PARTICLES, and the two halves +#: buy different things. Without `particles` there is no CHAIN -- +#: 'von' is the leading name piece again (given='von', +#: family='Richthofen') and _assign reports the fork. Without +#: `titles` the chain still happens (family='von Richthofen') but +#: this emitter's `all(title(x) for x in range(k))` guard fails, +#: so the REPORT is lost and _assign reports a fork about +#: 'Freiherr' instead. The merge sits outside that guard. +#: Its `particles_ambiguous` membership is irrelevant either way, +#: pinned by control 3 in the test below. +#: chained word ('von') PARTICLES_AMBIGUOUS. Drop it and the +#: chain still happens (family='von Richthofen') but no fork is +#: reported -- an unambiguous particle is not a decision. #: #: The distinction matters because this file used to assert #: `titles & particles_ambiguous` for the LEADING word, which is the @@ -309,49 +315,66 @@ def test_trailing_roman_numeral_reports_the_fork() -> None: #: subsets of `particles`, so it cannot empty `titles & particles` and #: cannot orphan this emitter. #: -#: These tests supply the memberships anyway rather than borrowing them, -#: because reachability is a property of the EMITTER, not of the shipped -#: word lists: a caller may configure the overlap themselves (`Lexicon` -#: asserts no invariant against it and construction warns about nothing). -#: What the SHIPPED vocabulary reaches is a separate claim, pinned where -#: it belongs -- the "Freiherr von Richthofen" row in tests/v2/cases.py, -#: which should fail loudly if #360 ever changes that parse. +#: Both roles are supplied rather than borrowed, because reachability is +#: a property of the EMITTER, not of the shipped word lists: a caller may +#: configure the overlap themselves, and no `Lexicon` invariant forbids +#: it -- constructing one emits no warning. Supplying only the leading +#: half would leave the tests coupled to #360 through the chained word, +#: which is the bug the first cut of this commit shipped. +#: +#: What the SHIPPED vocabulary reaches is a separate claim, pinned in +#: tests/v2/cases.py's "Freiherr von Richthofen" row. Note that row +#: tracks the PARSE, not the memberships: moving `freiherr` between the +#: particle halves leaves it green, and only a change to the leading +#: word's `titles`/`particles` membership, or to `von`'s ambiguous one, +#: moves it. _TITLE_PARTICLES = frozenset({"freiherr", "do", "st"}) +#: The words those tests CHAIN. Disjoint from the leading set on purpose +#: -- the two roles need different memberships, and holding them apart is +#: what keeps that legible. +_CHAINED_PARTICLES = frozenset({"von", "van"}) + def _overlap_parser(policy: Policy | None = None) -> Parser: - """A Parser whose lexicon spells every `_TITLE_PARTICLES` member as a - title, a particle AND an ambiguous particle, whatever the shipped data - says today. All three sets because these words appear in both roles - across the tests below -- 'Do St Johnson' chains `St`, which needs the - ambiguous membership, while `Do` leads and needs the other two.""" + """A Parser whose lexicon gives each word the memberships its ROLE + needs, whatever the shipped data says today: the leading words become + titles and particles, the chained words ambiguous particles. + + `particles` covers both because `_SUBSET_FIELDS` requires + `particles_ambiguous <= particles`; the leading words are deliberately + NOT made ambiguous, since that membership does nothing for them and + asserting it is how the wrong set got written down in the first place. + """ lex = Lexicon.default().add( titles=_TITLE_PARTICLES, - particles=_TITLE_PARTICLES, - particles_ambiguous=_TITLE_PARTICLES, + particles=_TITLE_PARTICLES | _CHAINED_PARTICLES, + particles_ambiguous=_CHAINED_PARTICLES, ) return Parser(lexicon=lex, policy=policy or Policy()) def test_the_chained_emitter_is_reachable_by_construction() -> None: """_group's PARTICLE_OR_GIVEN emitter fires when a piece that is both a - title and an ambiguous particle sits ahead of the chained particle. + title and a PARTICLE sits ahead of the chained particle. Asserted against a lexicon built here, so what it pins is the emitter - rather than today's word lists. The control carries the weight: the - SAME input, with the word a plain title instead, takes _assign's - leading-particle branch and reports the other detail -- so a passing - assertion below cannot be the parser doing what it would have done - anyway. The overlap is what routes to _group, not the title. - - If this test ever fails, the emitter really is gone or broken. An - empty `titles & particles_ambiguous` in the shipped vocabulary does - NOT fail it, and does not mean the emitter is unreachable. + rather than today's word lists. Three controls carry the weight, one + per membership the claim rests on: drop the leading word's `particles` + and the chain is gone; drop the chained word's `particles_ambiguous` + and the report is gone; drop the leading word's `particles_ambiguous` + and nothing moves at all -- which is the whole correction, since + asserting THAT membership is what this file used to do. + + If this test fails, the emitter is gone or broken, or one of the two + words this test builds has lost a membership it supplies itself. An + empty `titles & particles_ambiguous` in the SHIPPED vocabulary does + not fail it and does not mean the emitter is unreachable. """ word = "zzoverlap" - base = Lexicon.default() - overlap = base.add( - titles={word}, particles={word}, particles_ambiguous={word}) + base = Lexicon.default().add( + particles=_CHAINED_PARTICLES, particles_ambiguous=_CHAINED_PARTICLES) + overlap = base.add(titles={word}, particles={word}) text = f"{word} van Johnson" # the emitter, reached by construction @@ -381,6 +404,17 @@ def test_the_chained_emitter_is_reachable_by_construction() -> None: assert (quiet.given, quiet.family) == (chained.given, chained.family) assert quiet.ambiguities == () + # control 3 -- the leading word made ambiguous as well, which is the + # membership the deleted guard test asserted. Byte-identical to the + # treatment, fork included: it buys the emitter nothing. This is the + # executable half of the correction; without it the claim that + # `titles & particles` is the right set lives only in a comment, and + # a fixture supplying all three sets could never contradict it. + also_ambiguous = overlap.add(particles_ambiguous={word}) + same = Parser(lexicon=also_ambiguous).parse(text) + assert (same.given, same.family) == (chained.given, chained.family) + assert [a.detail for a in same.ambiguities] == [amb.detail] + def test_ambiguous_particle_reports_both_branches_of_its_fork() -> None: # "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: # "Dr. Van Johnson" is byte-identical to the bare "Van Johnson" # under every name_order, and the fork comes from _assign -- whose # detail DOES name the role, unlike the grouping-stage text above. - titled = Parser(policy=policy).parse("Dr. Van Johnson") - bare = Parser(policy=policy).parse("Van Johnson") + # through _overlap_parser as well: 'Van' has to be an ambiguous + # particle for _assign to report anything here, and that membership + # is exactly what #360 may move + p = _overlap_parser(policy) + titled = p.parse("Dr. Van Johnson") + bare = p.parse("Van Johnson") assert titled.title == "Dr." assert (titled.given, titled.middle, titled.family) == \ (bare.given, bare.middle, bare.family) diff --git a/tests/v2/test_properties.py b/tests/v2/test_properties.py index 586d0c4b..ac61a706 100644 --- a/tests/v2/test_properties.py +++ b/tests/v2/test_properties.py @@ -126,7 +126,16 @@ def test_a_leading_ambiguous_particle_is_reported_once_and_only_once( one is a fork -- so any reconstruction here would have to re-implement _group rather than check it. """ - lex = Lexicon.default() + # The 'Freiherr ' lead below has to be a title AND a particle, or it + # is transparent to the leading-particle exception (#367), every + # shape using it leaves _group's emitter for _assign's, and this + # sweep goes on passing with the fork count unchanged -- coverage + # lost silently, which is the failure this sweep is least able to + # notice about itself. Supplied rather than borrowed from the shipped + # vocabulary, for the reason test_parser.py's _TITLE_PARTICLES block + # gives; a no-op against today's data, and #360-proof against + # tomorrow's. + lex = Lexicon.default().add(titles={"freiherr"}, particles={"freiherr"}) # bound-given prefixes are excluded, not overlooked: 'abu' is both # an ambiguous particle and a bound given prefix, so whether it # forks depends on whether the bound join fired -- a second rule,