fix(pieces,assign): a comma-segment numeral is read by what precedes it (#430, #432) - #443
Open
derek73 wants to merge 4 commits into
Open
fix(pieces,assign): a comma-segment numeral is read by what precedes it (#430, #432)#443derek73 wants to merge 4 commits into
derek73 wants to merge 4 commits into
Conversation
Copy link
Copy Markdown
Owner
Sorry, something went wrong.
#430 and #432 are one boundary and land together. Inside a comma segment a roman numeral is read by WHAT STANDS BEFORE IT: Smith, PSM I given 'PSM' + suffix 'I' -> suffix 'PSM I' Smith, PSM I. given 'PSM' + suffix 'I.' -> suffix 'PSM I.' Smith, MD I title 'MD' + given 'I' -> suffix 'MD I' Smith, John V. suffix 'V.' -> middle 'V.' Smith, John V suffix 'V' -> unchanged, the boundary Behind a SUFFIX the numeral describes that suffix -- 'PSM I' is Professional Scrum Master level I -- so the run continues through it, period included, an initial there being no shape anyone writes. Behind a NAME the period is decisive: it marks an abbreviation, which is name material, so the numeral is the middle initial 'Smith, John B.' always was. Bare after a name it stays the generation (v1 parity, #144), which is why the test is the period and not the numeral. #430's cause was S2's initial veto in is_suffix_piece -- right after a name, wrong inside a credential run -- so the segment did not look like a run and fell to the given-and-suffix walk. One cause, two wrong answers: where the run's first word is TITLES vocabulary too the leading-title peel took it instead ('Smith, MD I' -> title 'MD', given 'I'), so a fix verified on PSM alone would have left that half broken and looked green. THREE readers, ONE reading. The gate that calls a segment a credential run, assign's router, and group's one-entry join must agree piece for piece; #429 shipped the inverse of its own fix by deriving that agreement twice. segment_suffix_reading returns the per-piece verdicts and all three read it (mechanisms.md#ONE-PREDICATE-PER-QUESTION). The intermediate state proved the point: with only the gate and the router converted, 'Smith, PSM I' rendered 'PSM, I' -- roles right, render split, the same tell as #429 one predicate along. Declined, and the v1 suite caught it: asking is_trailing_numeral_suffix for #432's half. It answers the period question, but also refuses a numeral behind an initial-shaped piece -- a no-comma rule, and the opposite of this path's parity. 'Chang, Andy C I' is first 'Andy', middle 'C', suffix 'I', and asking it here made the numeral a middle. The #401/#421 entry records the same fork declining to transfer to this walk. Only the period half applies. Verification: suite 5702, mypy and ruff clean, differential 0 unexplained at 1.4.0, 2.0.0 and 2.1.0. Swept across all three name_orders -- identical, these being segment reads rather than positional ones. Both clauses mutation-checked: disabling #430's kills three case rows, #432's one. The corpus is not blind to the class. Three of 1072 names carry a comma then a word then a numeral -- 'Berg, abdul V', 'Smith, John I', 'Smith, John V' -- all BARE, the form deliberately left alone, so they are evidence the boundary held. 'Berg, abdul V' cannot reach the walk at all: the bound-given join makes 'abdul V' one piece, which is why the LENIENT mirror #401/#421 declined does not collide here. Not done, recorded rather than left implicit: neither half emits an ambiguity, though the no-comma 'John Smith V' reports suffix-or-name for the same fork. #432 raises it as a secondary observation, and an emitter needs a trigger, a stage-ownership entry and a case row. Closes #430 Closes #432 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copy link
Copy Markdown
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## master #443 +/- ##
==========================================
+ Coverage 98.59% 98.60% +0.01%
==========================================
Files 45 45
Lines 3067 3091 +24
==========================================
+ Hits 3024 3048 +24
Misses 43 43 ☔ View full report in Codecov by Harness.
|
Sorry, something went wrong.
…scoped
Four reviewers. Two behavioural defects, both mine, and three documents
that described a cleaner rule than the code implemented.
STRICT WAS SILENTLY OVERRIDDEN. C1 says "strict mode vetoes
initial-shaped words"; segment_suffix_reading consulted no policy, so
under lenient_comma_suffixes=False an initial-shaped word began
counting toward the credential run -- 'Smith, PSM I.' went from given
'PSM' + suffix 'I.' to suffix 'PSM I.'. I checked strict while writing
this and got the wrong answer by comparing the wrong pair:
branch-lenient against branch-strict, which agree, instead of
master-strict against branch-strict. The reading takes `lenient` now
and only the numeral continuation consults it.
THE WALK'S GATE NEVER ASKED WHAT PRECEDES. The clause says the period
decides behind a NAME word and the run continues behind a SUFFIX, and
the gate read the piece alone -- so 'Smith, John PhD I.' went from
suffix 'PhD, I.' to middle 'I.', a numeral behind a suffix becoming an
initial, the opposite of the rule. 1,008 of one reviewer's 6,352
movers were that class. It asks now.
SCOPE. "A roman numeral" is neither a subset nor a superset of what
the code reaches: it sweeps in `2` (suffix vocabulary, one character --
'Smith, John 2.' is middle '2.'), misses X and IX (roman, not
vocabulary -- 'Smith, PSM X' never moved), and II/III/IV were never in
question at all, being two characters and so never initial-shaped and
never vetoed. The class is ONE-CHARACTER SUFFIX VOCABULARY, which is
the shape the code tests. rules.md, decisions.md and the release bullet
all said the other thing, and agreed with each other while disagreeing
with the parser -- writing the rule I meant instead of the one I built.
BLAST RADIUS, recomputed. The entry paired the post-PR population with
a pre-PR hit list that was itself short: master's corpus holds seven
names of the shape, not three, and NONE of them moves. Zero
pre-existing corpus names move; the only movers are the ones this
change adds as examples. So the gate's 0-unexplained is evidence about
the corpus, not about the change, and the case table is the whole
coverage. Said so.
Five rows, three of them for mutants that survived:
- family_comma_title_resets_the_credential_run ('Smith, PSM Dr. I').
The reset fires 60 times across the suite and no input observed it
until now -- deleting the line left all 5702 tests green.
- family_comma_strict_keeps_the_initial_veto, core-only: the knob has
no v1 spelling.
- family_comma_numeral_behind_a_suffix_is_not_an_initial.
- family_comma_run_numeral_after_a_period_abbreviation ('Smith, Jr. I')
-- the third route to the bug, named in the release log before it
had a row, and the one a writer actually produces.
- family_comma_run_numeral_after_a_split_credential ('Smith, Ph. D. I')
-- a run whose head is a MERGED piece, so the three-reader agreement
has a second structural pin and an edit to the PSM rows cannot
quietly unpin the render join.
Plumbing, from the type review: assign's gate folded into its router --
one call, no assert, and its "not a second derivation" comment becomes
true where it was describing a third evaluation. segment_holds_no_name
had no callers left and is gone; its docstring moved, including the
paragraph instructing the next reader to ask is_suffix_piece per piece,
which is the #430 bug. The veto helper no longer enumerates config
contents, and its unreachable len(piece) != 1 guard is documented as
defensive rather than left for the next person to try to kill.
Also: decisions.md#S2 does not exist -- the record is the #401/#421
entry under #P5. mechanisms.md's roster named the predicate that is no
longer shared and omitted the one that is.
Gates: 5727 passed, mypy and ruff clean, 0 unexplained at 1.4.0, 2.0.0
and 2.1.0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
codecov/patch and codecov/project both failed on one uncovered line: _pieces.py's `if len(piece) != 1: return False`, the guard the review established is unreachable through the parser -- a merged multi-token piece carries "suffix" in its PIECE tags, so is_suffix_piece claims it one branch earlier. The project has no codecov.yml, so any uncovered new line fails the patch check. Unreachable is not the same as removable: without the guard the helper would read piece[0]'s tags and answer for the FIRST TOKEN of a piece rather than for the piece. Returning False is the safe answer, so the branch stays and gets asserted directly, which is the only way it can be asserted at all. _pieces has had no unit-test module since #439 moved it out of _group; its predicates were reached end to end through the case table only. test_vocab.py is the precedent for testing a shared predicate directly. Four tests, each pinning something a name cannot reach: - the multi-token guard, built from the real Ph./D. merge rather than a hand-made piece, so it stays true to what group actually produces - the reading is positional and total -- one verdict per piece, in order, which is the invariant all three readers index by - the reading does not move when roles are assigned. This is the stability the shared-predicate design rests on and nothing stated it: group reads before assign and assign reads after, which is only safe because the predicates read tags and text, and assign writes roles. A stage that tagged during assignment would make the two readers disagree silently -- the drift #429 and #430 both are. - strict ends the run at the initial-shaped numeral, asserted at the predicate rather than through a parse nameparser/_pipeline/_pieces.py is now 100% covered and total uncovered lines drop from 44 to 43. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four angles. Reuse returned no findings; the rest found real work, and one of them found that a test added last commit was inert. THE GUARD TEST PINNED NOTHING. test_pieces asserted the len(piece) != 1 guard using the merged Ph./D. piece -- whose head carries vocab:suffix but NOT initial, so the assertion returned False through the TAG test and never reached the guard. Deleting the guard changed no result. The line was COVERED, which is why codecov cleared, and not PINNED, which is a different property. It now uses a piece headed by `V`, which the tag test would accept, so only the guard can produce the False -- verified by deleting the guard and watching it fail. after_suffix was exactly out[-1], maintained by hand at three sites, one of which deliberately omitted the assignment and so read as an oversight. Derived now; a fourth branch appending without assigning can no longer diverge silently, which is the drift shape this whole arc is about. The walk's gate becomes _reads_as_a_trailing_suffix, which settles four findings at once: it is evaluated where it is consumed rather than for every piece; `lenient` stops meaning two things sixty lines apart and then a third after rebinding; the cheapest test goes first, per the 7009fcf precedent this change had been citing while violating -- of 59 walk pieces reaching it, endswith('.') passes 7 where is_initial_shaped passes 9, so the period test first skips 52 of 59 regex calls; and the inert is_initial_shaped conjunct goes, with the docstring saying why it would be redundant rather than leaving the next reader to prove it. Also: lenient loses its default (a defaulted policy flag re-opens the door this arc's own entry says the first draft walked through); _group hoists `entry = reading or ()` instead of testing `reading is not None` per piece, where the test could never be False; no_name stops being a second name for `reading is not None`; the 95-char comment line, its third flagging, is rewrapped and the third topic spliced into the middle of someone else's argument moved down to its subject. Smith, Jr. I loses its row. The mutation matrix found it kills nothing Smith, MD I and Smith, Ph. D. I do not, and the distinction its note drew -- TITLES membership versus the period-abbreviation inference -- is how the two differed BEFORE the fix: both heads are suffix pieces now, so is_leading_title is consulted for neither. Its story folds into the MD row, which keeps a unique kill. TWO SKIPS, both recorded in decisions.md with the measurement: - Unifying the walk's neighbour test with the reading's flag. It looks obviously right and regresses: the walk assigns PSM to GIVEN in 'Smith, PSM I.', so a reading-style flag says "behind a name" and loses the strict parity restored two commits ago. The two sites ask different questions deliberately. The cost is 'Smith, John PSM I I.' moving -- two consecutive one-character numerals behind a credential, accepted rather than chased at the price of two correct pins. - The lazy restructure that would recover the walk's remaining ~1.4-2.2% on an all-family-comma workload. On a realistic mix the whole delta is -0.06%, and buying it back costs putting the 20-line declined- alternative comment above the `if` rather than inside the branch it explains. Worth knowing beyond this PR: test_benchmark could not have caught that 2%, since none of its workloads enters the post-comma walk -- the _POLICY_SHAPES lesson from its own docstring, recurring. Gates: 5726 passed, mypy and ruff clean, 0 unexplained at 1.4.0, 2.0.0 and 2.1.0, and _pieces/_assign/_group all at 100% line coverage. 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
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 #430. Closes #432.
They are one boundary, which is why they land together: inside a comma segment a roman numeral is read by what stands before it.
Behind a suffix, the numeral describes that suffix — PSM I is Professional Scrum Master level I — so the run continues through it, period included, an initial there being no shape anyone writes. Behind a name, the period is decisive: it marks an abbreviation, which is name material, so the numeral is the middle initial Smith, John B. always was.
Bare after a name it stays the generation (v1 parity, #144). That is why the test is the period, not the numeral.
One cause, two wrong answers
#430's root is rules.md#S2's initial veto in is_suffix_piece — right after a name, wrong inside a credential run — so the segment did not look like a run at all and fell to the given-and-suffix walk. Where the run's first word is TITLES vocabulary too, the leading-title peel took it by a different route instead (Smith, MD I → title MD, given I; Smith, Jr. I via the period-abbreviation inference). A fix verified on PSM alone would have left that half broken and looked green.
Three readers, one reading
The gate that calls a segment a credential run, assign's router that assigns its roles, and group's one-entry join that renders it must agree piece for piece. #429 shipped the inverse of its own fix by deriving that agreement twice, so segment_suffix_reading returns the per-piece verdicts and all three read it (mechanisms.md#ONE-PREDICATE-PER-QUESTION).
The intermediate state proved the point rather than assuming it: with only the gate and the router converted, Smith, PSM I rendered 'PSM, I' — roles right, render split, the same tell as #429 one predicate along.
Declined, and the v1 suite caught it
Asking is_trailing_numeral_suffix for #432's half. It does answer the period question — _ROMAN is anchored and admits no period — but it also refuses a numeral standing behind an initial-shaped piece, which is a no-comma rule and the opposite of this path's parity:
Asking that predicate here made the numeral a middle and broke that v1 test. decisions.md's #401/#421 entry records the same fork declining to transfer to this walk under LENIENT. Only the period half applies, so only the period half is used.
Verification
Swept across all three name_orders — identical, these being segment reads rather than positional ones.
Both clauses mutation-checked against a scratch copy: disabling #430's kills three case rows, #432's kills one. Neither guard is inert.
The corpus is not blind to this class
Of 1072 names, three carry a comma then a word then a numeral — Berg, abdul V, Smith, John I, Smith, John V — and all three are bare, the form this change deliberately leaves alone. They are evidence the boundary held rather than an absence of coverage.
Berg, abdul V cannot reach the walk at all: the bound-given join makes abdul V a single piece, which is why the LENIENT mirror #401/#421 declined does not collide here.
The moved names entered the corpora as C1's new examples and carry ledger entries at all three baselines — except #430's at 1.4.0, where fix(comma-family) lone post-comma piece routes to suffix/title, not first already claims the name and a second rule would be dormant.
Not done, recorded rather than left implicit
Neither half emits an ambiguity, though the no-comma John Smith V reports suffix-or-name for the same fork. The comma path has never reported it, #432 raises it as a secondary observation rather than a requirement, and an emitter needs a trigger, a stage-ownership entry and a case row.
🤖 Generated with Claude Code