Sweep the regex-sync pins over every differential ledger - #349
Open
derek73 wants to merge 14 commits into
Open
Conversation
The three toml pins each hardcode expected_since_1.4.0.toml. Introduce the glob they will sweep, with a vacuity guard: a parametrize over an empty list generates zero tests and passes silently, which is the failure mode this module exists to break.
expected_since_2.0.0.toml carries two hand copies of _policy._SCRIPT_RANGES that no test could see: the pin named the 1.4 file by filename. Parametrize over the glob instead, with a per-file floor recorded in _SPAN_BEARING_FLOORS so a new baseline's ledger fails as unrecorded rather than passing unmeasured. The floor is >= rather than ==: a new span-bearing rule is pinned by discovery the moment it exists, so the number's only job is catching a copy that fell out of reach. The old global >= 3 was already one behind -- the 1.4 ledger has three compound rules, not two.
The selector picked rules by the literal '#271'/'#272' substrings and asserted uniqueness; #332 broke it by adding a second matching rule to expected_since_2.0.0.toml, and the test's own docstring told rule authors to avoid those substrings in compound slugs to keep it working. Its equality check was already fully subsumed by the span-bearing sweep -- the canonical rule is itself span-bearing -- so only two assertions were load-bearing: that a Script joining _SCRIPT_RANGES forces a written decision, and that a sanctioned extra stays unclassified. Both are facts about the table rather than about any ledger, so they move to a test that names no file. A selector break can no longer take the decision gate out as collateral, and slugs may now cite #271/#272 freely.
It asserted exactly one cjk-delimited-nickname rule in the hardcoded 1.4 file. The 2.0 ledger has none, so neither a per-file nor a global count is a true invariant; what is true is that every such rule carries the sanctioned trigger set, and that at least one exists somewhere. Also folds away a dead branch: the two spellings the old check compared against were the same string, since a unicode escape in a non-raw literal is already the character it names.
expected_since_2.0.0.toml hand-copies the CJK members of SUFFIX_NOT_ACRONYMS and, separately, GLUED_HONORIFICS -- which had no pinned copy anywhere in the tree. Neither was checked: the pin named the 1.4 file, and read its alternation by asserting the rule starts with '(?:^| )(?:' and ends with ')$'. Neither 2.0 rule is anchored that way, so globbing alone would have failed on shape rather than on content. Replace the shape parser with a generic extractor over (?:a|b|c) groups carrying a script-classified member, and add a roster declaring which constant each rule mirrors. A roster rather than an inference because GLUED_HONORIFICS is a subset of SUFFIX_NOT_ACRONYMS, so 'equals one of the two' would let a narrowed spaced rule pass by matching the other.
The 2.0 ledger's comment told readers its copies were unpinned and named the selector rework as a prerequisite; AGENTS.md's release step 8 told the author to land #333 before opening the next cycle's ledger. Both are done, so both now say what holds and what the author must still do: add the new file to _SPAN_BEARING_FLOORS.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #349 +/- ##
=======================================
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:
|
The recorded count was identity-free, so a copy could leave discovery while an unrelated span-bearing rule was added and the total would hold steady with a hand copy silently unpinned. Measured: rewriting the #298 rule's class in literal characters and appending one rule passed under the count and fails now. Two more things fall out of naming them. The floor's >= admitted a ledger recorded as 0 that actually carried CJK rules -- a permanently inert reach guard, and AGENTS.md now tells release engineers 0 is sometimes right. And the roster gains the staleness direction its two siblings in this module already had: a tag matching no rule fails, so a renamed or deleted rule cannot leave its entry behind. Rules are named by the leading fix(...)/feat(...) tag of their issue, with a uniqueness check so a shared tag reads as a naming-scheme break rather than as a disappearance.
Both are notation blind spots that let a copy sit in a ledger while the pin that should read it looks straight past. _declared_spans reads one spelling, so set equality against the table only ever pinned what was written as an escaped span. Anything appended in another notation rode along unchecked -- and the file's own convention mixes spellings, so this is not hypothetical. The widening direction is where it bites: "[CJK...a-z]" passes both the equality and compare.validate_rules' sentinel probe, and lets the rule claim every Latin diff in the corpus as intended. A span-declaring class must now hold nothing but spans; classes carrying no spans are a separate decision surface and stay out of scope. _ALTERNATION required a literal "(?:", so a rule written "(씨|님|先生)" carried a hand copy the honorific pin could not see -- silent unpinning, the failure this module exists to prevent. It now matches plain capturing groups too, guarded by (?!\?) so lookarounds do not parse as alternations of their own syntax. Same three alternations found today. Also: say what an unmatched roster key actually means, since an unparseable alternation reports identically to a renamed rule, and read name_regex with .get() in the nickname sweep so a rule without one asserts instead of raising KeyError.
Three comments told rule authors to keep the literal #271/#272 strings out of their slugs, to protect a pin that selected the canonical CJK rule by those substrings and asserted uniqueness. That pin is gone (#333) and the sweep finds rules by their character class. This is the file authors actually edit when writing a rule, so it was the copy of the instruction that mattered -- the test module's own docstring already says the constraint is lifted, but nobody writing a ledger rule has reason to open it.
Four claims that review falsified by measurement:
- The module docstring's inventory ("the last three tests ... two to a
TOML file") was already off by one on master and this PR widened it.
Four tests read the ledgers now, and they are not the last three.
- _SANCTIONED_EXTRAS said "both span pins below read this set". There is
one span pin now; the other reader is the membership guard, which is
exactly the split this PR argues for elsewhere.
- test_ledger_glob_is_not_empty said an empty parametrize "generates
zero tests and passes vacuously". Measured: pytest emits one SKIPPED
test. The conclusion -- no failure signal -- stands; the mechanism was
wrong.
- The nickname test said neither a per-file nor a global `== 1` is true.
A global one IS true today, just for the wrong reason. It is the wrong
invariant, not a false statement.
Also moves the astral-block rationale onto _expected_bmp_spans, which is
the code it explains, and drops the U+FF65/U+00B7 argument from
test_script_ranges_membership_is_decided rather than keeping a second
copy of what _SANCTIONED_EXTRAS already says 60 lines above. Two copies
of one rationale is two things to keep in sync, which is the failure
this module exists to prevent.
Adds the scope note the nickname pin was missing: it is a decision
surface, not a sync pin, and _NICKNAME_DELIMITERS cannot derive from
Policy.nickname_delimiters because the class also carries the nakaguro
separators, which delimit nothing.
Step 8 named _SPAN_BEARING_RULES only. A new ledger's CJK honorific rule carries a new slug by construction, so no _HONORIFIC_SOURCES key covers it and the honorific pin hard-fails as undeclared. Both reviewers reached this independently by building a realistic next-cycle ledger and running it. The failures are loud and self-describing, so this is checklist completeness rather than a silent hole -- but step 8 is the repo's "everything you must do" list, and it is read exactly once per release, at the moment it matters.
Re-review measured every factual claim in the previous five commits.
Eleven held; these did not.
The nickname scope note said removing a delimiter pair from the config
"fails the behavior tests in tests/v2/pipeline/". Measured: that
directory passes 240/240. The catch is tests/v2/test_cases.py, on the
cjk_white_corner_bracket_nickname row. Pointing the next author at a
directory that stays green is the exact defect this series set out to
fix, committed by the series itself.
_ALTERNATION's comment said a "|" inside a character class surfaces
through the STALE roster check. It does not: the member split is a
plain str.split("|"), so "[a|b]" is MISread as members "[a" and "b]",
the rule keeps matching its key, and it fails the declared-vs-expected
equality instead. Only a nested group or a paren inside a class routes
to STALE. The STALE failure message carried the same wrong cause and
would have sent a reader hunting for something that cannot produce it.
AGENTS.md claimed a new ledger's honorific slug is new "by
construction". Nothing constructs that: a retroactive ledger repeats an
older one's rules, and fix(#271/#272/#298) is in both files today --
which is why _SPAN_BEARING_RULES records per file in the first place.
Also: "this file's own convention" meant the ledgers while sitting in a
test module docstring one sentence away from a quote using the same
phrase for the toml; the astral-block note said "the comment at the
rule" when two rules carry the class and only 1.4's explains it; the
_NICKNAME_DELIMITERS rationale said "cannot derive" when the honest
reason is that the class is a deliberate subset-plus-nakaguro, not an
impossibility; and the .get() introduced last round now says why it is
there and what it catches.
Adversarial review defeated both halves of the previous round. A rule's class respelled in literal characters means the identical regex while dropping out of discovery, leaving its hand copy unpinned. Naming rules instead of counting them was supposed to catch that, and does -- unless the edit also adds a rule answering to the same name. That is not exotic: the 1.4 ledger already has two rules tagged feat(#269). No name-keyed roster can survive this, because a set of names cannot see that two different rules answer to one name, and neither counting nor full issue strings changes that. So close the hatch rather than improve the roster. A classified codepoint spelled literally inside a character class is now refused across every rule in every ledger -- not only the discovered ones, since reaching the undiscovered ones is the whole job. The nakaguro separators are sanctioned: U+30FB sits in the katakana block by assignment while functioning as punctuation, which is why the delimiter rules spell it as itself. Separately, appending "|[A-Za-z]" at depth 0 widens a span-bearing rule exactly as appending "a-z" inside its class would, and clears both layers meant to stop it -- the span equality sees an unchanged class, and validate_rules' sentinel probe passes because Хосе Сантос fails to match. Span-bearing rules must now have no depth-0 "|"; an alternation that is genuinely wanted goes in "(?:...)". Also adds the file-key staleness check the roster was missing (its per-ledger sweep runs over _LEDGERS, so an entry naming a deleted file was never visited), and corrects the _ALTERNATION comment again: the unreadable shapes split across TWO mechanisms, not one, and which message you get depends on whether the misparsed members are classified.
The message read as though every ledger rule needs a fix(...) tag. Only rules the roster has to name do, and one 1.4 rule has none today -- harmless while it declares no script span, and a tag-it-then problem if it ever does.
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 #333.
tests/v2/test_regex_sync.pypinned the differential harness's hand copies by hardcodingexpected_since_1.4.0.toml, so the four hand copies added toexpected_since_2.0.0.tomlin #332 were unchecked. This makes the pins sweep everyexpected_since_*.toml.The four copies, now pinned
expected_since_2.0.0.toml:63_policy._SCRIPT_RANGEStest_every_span_bearing_rule_matches_the_script_ranges[expected_since_2.0.0.toml]expected_since_2.0.0.toml:229_policy._SCRIPT_RANGES, inside the #298 lookaheadexpected_since_2.0.0.toml:162SUFFIX_NOT_ACRONYMStest_differential_honorific_rules_match_their_vocabularyexpected_since_2.0.0.toml:116GLUED_HONORIFICSGLUED_HONORIFICShad no pinned copy anywhere in the tree before this.What changed, and why it wasn't a one-line glob
The
#271/#272selector is retired.test_differential_cjk_rule_matches_the_script_rangesselected rules by issue substring and asserted uniqueness; the 2.0 ledger has two matching rules, so #332 broke it. Its equality check was already fully subsumed by the span-bearing sweep — the canonical rule is itself span-bearing — so only its two table-level guards were load-bearing. Those move totest_script_ranges_membership_is_decided, which names no ledger. The sweep now owns "every hand copy equals the table" and that test owns "the table didn't change shape without a decision", so a selector break can't take the decision gate out as collateral. Rule authors may now cite #271/#272 in compound slugs freely — the old docstring forbade it.The span-bearing floor is per-file and recorded.
_SPAN_BEARING_FLOORSmaps each ledger to its known count, and membership is asserted — a new baseline's ledger fails as unrecorded rather than passing unmeasured. It's>=, not==: a newly added span-bearing rule is pinned by discovery the moment it exists, so the number's only job is catching a copy that fell out of reach.The honorific pin needed more than a glob. It read its alternation by asserting the rule starts with
(?:^| )(?:and ends with)$. Neither 2.0 rule is anchored that way, so pointing the old parser at the file would have failed on shape, not content. Replaced with a generic extractor over(?:a|b|c)groups carrying a script-classified member, plus_HONORIFIC_SOURCESdeclaring which constant each rule mirrors. A roster rather than inferring the source, becauseGLUED_HONORIFICS <= SUFFIX_NOT_ACRONYMSis asserted atconfig/suffixes.py:848— the sets are nested, so "equals one of the two known sets" would let a spaced rule that silently narrowed to exactly the glued set pass by matching the other member.The nickname delimiter check sweeps too. It asserted exactly one such rule in the hardcoded file; 2.0 has zero, so neither a per-file nor a global count is a true invariant. Also folds away a dead branch — the two spellings it compared against were the same string, since a unicode escape in a non-raw literal is already the character it names.
Two corrections to the issue text
Both re-measured against the working tree on 2026-08-07:
assert len(checked) >= 3floor and its comment ("the canonical rule plus both compound rules") were already one behind: the 1.4 ledger has three compound rules, not two.Verification
Each pin was demonstrated non-inert by mutating its source and confirming failure, not by asserting it works:
教授from bothSUFFIX_NOT_ACRONYMSandGLUED_HONORIFICSnow fails the honorific pin. Removing it fromSUFFIX_NOT_ACRONYMSalone trips the subset assert at import instead, which proves nothing about the pin, so the stronger variant was run._SPAN_BEARING_FLOORSmembership and floor assertions, the ledger-glob vacuity guard, and both table guards were each shown failing.Full suite: 3048 passed, 41 skipped, 11 xfailed.
mypyandruffclean.Scope
Tests and comments only — no change to
nameparser/ortools/differential/compare.py.AGENTS.mdrelease step 8 previously said to land this before opening the next cycle's ledger; it now says the new ledger's copies are pinned automatically, but the file must be added to_SPAN_BEARING_FLOORS.Review round (5 commits after
b09ccf4)A three-agent review (code quality, test coverage, comment accuracy) found no critical correctness issues but did find real gaps. Four findings converged across two reviewers independently.
_SPAN_BEARING_FLOORScounts became_SPAN_BEARING_RULESslug sets. A count is identity-free: a copy can leave discovery — rewritten as literal characters, say — while an unrelated span-bearing rule is added, holding the total steady with a hand copy silently unpinned. Measured passing under the count; fails now. Naming the rules also closes the>=hole (a ledger recorded as0that actually carries CJK rules was accepted, its reach guard inert forever, whileAGENTS.mdtells release engineers0is sometimes right) and gives the roster the staleness direction its two siblings in the module already had.Two notation blind spots closed.
_declared_spansreads one spelling, so appendinga-zliterally to a pinned CJK class passed both the span equality andcompare.validate_rules' sentinel probe — letting the rule claim every Latin diff in the corpus as intended. A span-declaring class must now hold nothing besides spans. Separately,_ALTERNATIONrequired a literal(?:, so a rule written(씨|님|先生)carried a copy the honorific pin could not see; it now matches plain capturing groups, guarded so lookarounds don't parse as alternations of their own syntax. Same three alternations found today.Three stale instructions removed from
expected_since_1.4.0.tomltelling rule authors to keep#271/#272out of their slugs to protect the pin this PR deletes. That's the file authors actually edit when writing a rule.Prose corrections, each falsified by measurement: the module docstring's test inventory (already off by one on master, widened here);
_SANCTIONED_EXTRAS' "both span pins below" (there is one now, by this PR's own design); "an empty parametrize generates zero tests and passes vacuously" (pytest emits one skipped test — the conclusion stands, the mechanism was wrong); and the nickname docstring's claim that a global== 1is false (it is true today, just the wrong invariant).AGENTS.mdstep 8 now names both manual steps. A new ledger's CJK honorific rule carries a new slug by construction, so no_HONORIFIC_SOURCESkey covers it and the pin hard-fails as undeclared. Loud rather than silent, but step 8 is the repo's "everything you must do" list.Review also surfaced a pre-existing defect outside this PR's scope, filed as #350: the
fix(#274)maiden rule matchesborn, which is in no config constant, so a realmaiden/middle/familyregression on "Max Born" is classified as intended.🤖 Generated with Claude Code