Pin the maiden rule to its vocabulary, and open the 2.1 ledger - #351
Merged
Conversation
The fix(#274) rule's alternation is a hand copy of MAIDEN_MARKERS that nothing checked. It offered 'born', which is in no config constant and never was, in v1 or v2 -- so that alternative could not correspond to a real #274 change. What it could do is claim any name containing "born", with fields maiden/middle/family, and absorb a genuine regression: the rule matches 'Max Born'. The pin is not the honorific pin's set equality, because these members are regex fragments rather than entries -- n[ée]e covers two markers and geb\.? covers one. It asserts the direction that matters instead: every alternative must match at least one entry the vocabulary ships. The covered set is recorded rather than equated, since widening to markers no corpus name exercises buys nothing and 旧姓 already has its own rule in the 2.0 ledger. Removing 'born' changes no classification -- no name in any of the three corpora contains it, which is also why it was never caught. English is absent from the vocabulary deliberately (it borrows née); promoting 'born' would be a behavior change with a real cost, since 'Bertha Born Smith' loses its family name to the marker.
A ledger is created the day its baseline is released, before that
cycle has produced a single behavior change, so it is empty and correct
until the first one lands. Two things refused that state: the
shipped-ledgers test asserted every file carries rules, and _rules read
["change"] where compare.py has always used .get("change", []).
The carve-out is exactly the ledger DEFAULT_BASELINE names. Older ones
are history, and history is not empty, so emptying one is still caught
-- and the test now also asserts the open cycle's file exists at all,
since _allowlist_for hard-errors on a bare run without it.
Release step 8 was outstanding from 2.1.0: the baseline still named 2.0.0 and no expected_since_2.1.0.toml existed, so a bare compare.py run measured against a minor further back than it reported. The new ledger is empty and correct that way. nameparser/ is byte-identical to the v2.1.0 tag, so this cycle has produced no behavior change to classify yet. It has to exist anyway, because _allowlist_for treats a missing ledger as a hard error and the baseline cannot advance without it. Verified end to end rather than by the summary line alone. Against 2.1.0 the harness reports 751 names, 0 intentional, 0 unexplained -- which is also what a harness comparing the installed wheel against itself would print, so that run cannot stand on its own. Against 2.0.0 it reports 89 intentional and 0 unexplained, which is the discriminator: the comparison is real, and the empty result is a measurement rather than a broken run. _SPAN_BEARING_RULES records the file with an empty set, which is the enrollment AGENTS.md step 8 prescribes and this is its first exercise.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #351 +/- ##
=======================================
Coverage 98.48% 98.48%
=======================================
Files 41 41
Lines 2845 2845
=======================================
Hits 2802 2802
Misses 43 43 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Review defeated the pin three ways, each measured through the harness's own classify() against the real ledger. It had no discovery direction. It iterated roster keys and skipped every rule that matched none, so a hand copy under an unrostered tag was pinned by nothing -- and one already was: the MA/DO acronym rule copies SUFFIX_ACRONYMS_AMBIGUOUS and no test could see it. It now iterates alternations, so an undeclared one fails; alternations that copy no vocabulary are declared as such rather than skipped. Appending "|[A-Za-z]" at depth 0 passed. The pinned alternation then governs one branch of an unchecked whole, and a family-only regression would be labelled fix(#274) on 675 of 783 corpus names instead of 4. _top_level_alternation already existed for exactly this -- it was applied one function away, inside the span-bearing sweep only. A member of "[a-z]{3}" or ".{3}" passed, because fullmatch against the vocabulary bounds what a member matches within 17 entries and says nothing about what it matches in a name -- and every entry this rule needs happens to be three characters. 320 of 783 names. Members are now probed against ordinary name fragments too. Also: a NamedTuple, since the roster's two set-shaped fields are not the same kind of thing and the positional tuple said nothing about which was the source of truth; re.error re-raised as an assertion naming the ledger, rule and member, since a mis-split alternation can manufacture an invalid fragment; and the vacuity guard its twin has.
Two defects in how the empty ledger was shipped, both found by trying
to use it.
`change = []` blocks the next step the file's own comment asks for:
TOML forbids appending a [[change]] table to a statically defined
array, so adding the first 2.2 rule raises TOMLDecodeError pointing at
the new rule rather than at the line that caused it. The line was never
needed -- both readers use .get("change", []), so an absent key already
means an empty ledger. Removed, and appending now works.
The carve-out that lets this one ledger be empty also lets it be
broken. A mistyped table header -- [[changes]], [[rules]] -- reads as a
legitimately empty open cycle everywhere: every sweep gets zero rules
and passes while the author believes they shipped a rule. The other
ledgers are protected by having to be non-empty, so this is the one
place it needs saying: nothing but `change` may be defined at the top
level of the open cycle's file.
The existence assertion moves out of test_validate_rules_accepts_the_
shipped_ledgers into a test named for what it checks. It is a fact
about the DEFAULT_BASELINE-to-ledger coupling in compare.py, not about
whether validate_rules accepts the shipped rules, and someone half way
through release step 8 will look for it by name.
Review measured every factual claim in the last round. These did not survive. "It covers the four the corpora exercise" was wrong on both halves. The rule's members reach 4 of the 17 MAIDEN_MARKERS entries; the corpora contain 3 markers; only 2 are in both sets. The next clause then contradicted the first -- "widening it to markers no corpus name contains would buy nothing" -- when nee and roz are exactly such markers and were already in. "旧姓 has its own rule in the 2.0 ledger, so claiming it here too would make two rules answer for one diff" named the wrong file and a mechanism that cannot happen: compare.py loads exactly one ledger per run, so rules in different ledgers never compete. The rule that could actually be shadowed is fix(cjk-maiden-marker), 20 lines below in THIS file, which sorts after fix(#274) within the name_regex tier. "English is absent from the vocabulary on purpose: it borrows née" was invented. maiden_markers.py has a "Deliberately absent" note and it names Polish z domu and Scandinavian f., not English -- and 'nee' is in the vocabulary, which tests/v2/cases.py calls the spelling English writes most often. The note now points at the real one. _rules' comment described a key-absent shape the shipped ledger contradicted; the ledger no longer defines `change` at all, so the comment is now true, and it says where that leniency is policed. Also sweeps a pre-existing leftover from the Role-vocabulary migration in the same file: fields "maiden/middle/last" and "flips `first`" for a rule that ships given/middle/family. AGENTS.md release step 8 gains the third roster, and drops the claim that the sweep needs no discovery. tools/differential/README.md's quick-start hardcoded --baseline 2.0.0, which is now two minors back; the bare invocation is the release-time run.
Four review rounds found the same defect in four costumes: a hardcoded
slug, then a hardcoded count, then a hardcoded probe list, then a
hardcoded nesting depth. Each guard closed the spelling that had just
been demonstrated and left the next one open, because every one of them
checked regex SYNTAX as a proxy for what a rule claims.
The last round made the pattern undeniable. "[acdf-uw-z]{3,}" walks
through all eight probe strings and claims 563 of 783 corpus names --
wider than the 320-name attack the probes were added to stop, and
derivable rather than lucky, since every entry the rule needs is three
characters so a member must accept one and is free everywhere else. And
a copy written as a character class rather than an alternation was not
undeclared but unseen: no roster demanded a key, nothing fired, 320
names claimed.
So stop asking how a rule is spelled and ask what it reaches. The
ledgers exist to explain diffs on the corpora and nothing else, so the
corpus is the whole population a rule will ever be asked about -- and
it answers without parsing anything, just a regex search.
Three guards, all corpus-derived:
- A span-bearing rule must claim no corpus name carrying an
unclassified script. The depth-0 test could not see "(?:CJK|[A-Za-z])"
hiding its pipe one level down; this does, because it never looks at
the pipe. Both are kept -- the syntactic one gives a clearer message
for the naive spelling and covers a widening toward a script the
corpora do not happen to contain.
- Every corpus fragment a vocabulary member matches must BE an entry,
normalized (the corpus writes "geb.", the config stores "geb") and
exempting only that member's own vocabulary. Replaces the probe
tuple, which could never be more than a spot check.
- Every rule claiming a role in _FIELD_VOCABULARIES must claim only
names that carry that vocabulary. Keyed on `fields`, not on the
regex, which is what closes the unseen-copy hole: a rule that does
not claim `maiden` cannot be a maiden change, so no notation escapes
it. It also reaches fix(cjk-maiden-marker), whose regex is the bare
literal 旧姓 and which no roster in this module could see.
Verified by replaying every attack from all four rounds: depth-0
widening, wrapped widening, [a-z]{3}, [acdf-uw-z]{3,}, geb[^o]*, the
class-with-no-alternation rule, and 'born' itself. All fail; the
shipped rules stay green; removing an entry from MAIDEN_MARKERS still
fails.
Three rounds, three wrong versions of the same explanation, while the correct one sat 30 lines below in the sibling rule's own note. Round 2 said 旧姓 "has its own rule in the 2.0 ledger, so claiming it here too would make two rules answer for one diff" -- wrong file, and a mechanism that cannot happen, since compare.py loads one ledger per run. Round 3 said the 旧姓 rule "sorts AFTER this one, so widening here would shadow it" -- the sort order is right and the consequence is wrong: classify() needs the diff's fields to be a subset of the rule's, and both 旧姓 corpus names diff on `given`, which fix(#274) does not claim. Widening it to 旧姓 is inert, not shadowing. Which is exactly what fix(cjk-maiden-marker)'s own note has said all along: #274's fields stop short of `given`, and that is why the separate rule exists. Both copies now point there instead of inventing a third mechanism. Also: "the other 13" was 12 once 旧姓 is accounted for separately; the roster comment said "this rule" three times while the roster now holds two; the member-loop docstring counted three assertions where two are per-member and one is per-rule; and "both readers" was four call sites across three files. AGENTS.md promised that "a copy the rosters cannot account for fails loudly rather than going unpinned". That is true of the field-keyed corpus check and false of the two syntax-keyed rosters -- a hand copy that is not an alternation or a span class is invisible to them, as fix(cjk-maiden-marker)'s bare 旧姓 literal was until this branch. The step now says which mechanism covers what.
Round five defeated the round-four guards twice more, and the pattern in the FIXES is now as clear as the one in the defects. Each guard was scoped to a category -- a span class, an alternation's members, a `maiden` field, a member's reach -- and each attack simply moved to a rule outside whichever category the last fix covered. feat(#273) is a nickname-delimiter character class: no span, so the span guard skips it; no alternation, so the vocabulary roster never discovers it; no `maiden` field, so the field guard skips it. Append A-Za-z and it goes from claiming 6 corpus names to 668, with the whole 2232-test suite green. And deleting the acronym rule's delimiter classes -- leaving its members untouched -- takes it from 0 to 193, because the members really do reach those names on their own; the narrowing lived in the context around them, which nothing measured. The categories are the test's, not the ledger's. What every rule shares is how much of the corpus it claims, and no widening can change what a rule matches without changing that. So _CORPUS_CLAIMS records the number per rule, per ledger, and refuses to let it move quietly. It is deliberately dumb. It knows nothing about vocabularies, scripts or roles and cannot say whether a number is right -- only that it moved. The specific guards stay, because they explain WHY a rule may claim what it claims and their messages are the ones worth reading; this is the backstop none of them could be. Keyed on the full `issue`, since the 1.4 ledger has two rules tagged feat(#269) and a tag-keyed roster cannot tell them apart. Verified by replaying every attack from all five rounds: both of round five's, the nested-and-widened acronym, [acdf-uw-z]{3,}, depth-0 |[A-Za-z], the class-with-no-alternation maiden rule, and 'born'. All fail. The counts move when the CORPORA move too, which is the intended cost -- a corpus name landing under an existing rule changes what that rule explains, and should be read once rather than absorbed. Also replaces the module's own "> 500" corpus floor with compare.py's per-file _CORPUS_FLOORS. A total cannot see a file vanish: emptying corpus_issues.jsonl left 583 names and every test green, silently dropping a quarter of the population every guard here measures.
Round six defeated the count three ways, and the root of all three is that a count of regex matches is not what a rule claims. classify() narrows on `name_regex` AND `fields`. _CORPUS_CLAIMS recorded only the first, so widening a rule's roles moved nothing it could see: the comma rule kept its `,` regex and its 236 names while going from explaining 6 of the corpus to 242. The fields-only rule -- no name_regex at all, the most unbounded shape validate_rules permits -- was not recorded at all, and widening it to six roles absorbed 751/751 on every diff shape not touching `title`. A count is also identity-free, the weakness this module rejects at _SPAN_BEARING_RULES. Swapping feat(#273)'s delimiter class for a single accented letter held the count at 6 while claiming six entirely different names. And it was keyed on `issue` in a dict comprehension, so a duplicate silently kept the last rule written. Nothing asserts issue uniqueness -- validate_rules wants only a non-empty string -- and the collision is worse than it sounds: _sorted_rules is stable, so FIRST in file is the position classify reaches, and first in file is exactly what the comprehension discards. Coverage by file order is no coverage. So record a _Claim: the regex's corpus reach, the sorted roles, and a digest of which names. Plus the uniqueness assert, which the two substring-keyed rosters lean on as well. Also corrects the framing, which had the layers backwards. This is a change DETECTOR, not an enforcer -- inert for a new rule, and its own message invites the re-record that defeats it. The member and vocabulary guards are the walls, because they judge a rule wrong at any time including at recording time. This catches what none of them is scoped to see and holds it still long enough to be read. Deletes the duplicate corpus-floor test, which restated two tests test_differential.py already owns and re-copied their compare.py loader to do it. What is local to this module is the deduplicated population its guards measure, so that is what stays.
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.
Closes #350, and clears release step 8, which was outstanding from 2.1.0.
Pin the maiden rule, drop
born(#350)expected_since_1.4.0.toml'sfix(#274)rule hand-copiesMAIDEN_MARKERSand nothing checked it. It offeredborn, which is in no config constant and never was, in v1 or v2 — traced to1748493, the harness's first commit. So that alternative could not correspond to a real #274 change; what it could do is claim any name containingbornwithfields = ["maiden", "middle", "family"]and absorb a genuine regression. The rule matchesMax Born.The pin is deliberately not the honorific pin's set equality, because these members are regex fragments, not entries —
n[ée]ecovers two markers,geb\.?covers one, so there is no set to compare:旧姓already has its own rule in the 2.0 ledger, so folding it in would make two rules answer for one diff. Recording still catches removal: drop an entry a member covers and the set shrinks.Dropping
bornchanges no classification — no name in any of the three corpora contains it, which is also why it was never caught.On promoting it instead: English is absent from
MAIDEN_MARKERSon purpose, since English borrows née. Addingbornis a behavior change with a real cost, measured:A trailing
Bornis safe;Bornin non-final position is not — which is exactly where a birth surname carried as a middle name sits. Left as a possible future release, not an issue.Open the 2.1 cycle
DEFAULT_BASELINEstill said2.0.0and noexpected_since_2.1.0.tomlexisted, so a barecompare.pyrun measured against a minor further back than it reported. The two are coupled —_allowlist_fortreats a missing ledger as a hard error — so the baseline could not advance without the file.The new ledger is empty and correct that way:
nameparser/is byte-identical to thev2.1.0tag, so this cycle has produced nothing to classify yet. That required one carve-out:test_validate_rules_accepts_the_shipped_ledgersasserted every ledger carries rules. It now permits exactly the ledgerDEFAULT_BASELINEnames — older ones are history, and history is not empty, so emptying one is still caught. The test also now asserts the open cycle's file exists at all.Verified end to end, not by the summary line
Against 2.1.0 the harness reports
751 names; intentional diffs: 0; unexplained: 0. That is also what a harness comparing the installed wheel against itself would print, so it cannot stand alone. The discriminator:The comparison is real, and the empty result is a measurement rather than a broken run.
_SPAN_BEARING_RULESrecords the new file with an empty set — the enrollment AGENTS.md step 8 prescribes, and this is its first exercise.Scope
Tests, tooling and ledger comments. One behavior-adjacent line in
compare.py(DEFAULT_BASELINE); no change tonameparser/.🤖 Generated with Claude Code