Repository navigation
Fix the review findings on two_family_names (#875) - #878
Merged
Merged
Conversation
A review of the merged #875 found five things: - Policy.two_family_names' docstring, which autodoc renders as the API reference, still gave "the mother's name first" as why Portuguese and Brazilian names break and named none of the refusals the review commits added; it now states them and the measured cause. - The rules/O6 citation comment lacked its decisions/O6 (#754) pointer. - The hyphen set missed the figure dash and the small and fullwidth hyphen-minus, so "Ana María García－López" took 'María'; it keeps family 'García－López' now, with a rules/O6 example and a case row. No PSC record holds any of the three, so decisions/O6's figures stand. - The role partition re-implemented _idx. - A PolicyPatch comment line was left overlong. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #878 +/- ##
==========================================
- Coverage 99.07% 99.07% -0.01%
==========================================
Files 47 47
Lines 4779 4774 -5
==========================================
- Hits 4735 4730 -5
Misses 44 44 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The fix commit's review found the widened hyphen class mostly
unpinned: cutting it to three members passed every test. A row per
member now checks "Ana María García{h}López" keeps the hyphenated
family, against a written-out list a roster test holds the regex to,
so dropping or adding a member fails. The minus sign, a word
processor's substitute for a hyphen, joins the class; em dashes are
punctuation between words and stay out. No PSC record holds it.
The Policy docstring had dropped English names from the cost it states
and with them the mixed-population warning; both are back, and it says
a spaced Han or Hangul name moves under a declared FAMILY_FIRST.
decisions/O6 names the widened class and that its figures stand.
Co-Authored-By: Claude Opus 5.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.
Follow-up to #875. A review of the merged change found no wrong parse in the rule as designed, but five smaller problems, all fixed here:
Policy.two_family_namesdocstring. Autodoc renders it as the API reference. It still gave "the mother's name first" as why Portuguese and Brazilian names break, and named none of the refusals the Add a Policy switch for names with two family names (#754) #875 review commits added (an initial written alone, a two-word part, a name its script ordered). It now states them, and the cause: one family name behind two given names, which is most English and Argentine names of three words or more and many Portuguese and Brazilian ones as filed, so on a mixed population the switch costs more than it mends. It also says a spaced Han or Hangul name moves under a declaredFAMILY_FIRST.decisions/O6 (#754)pointer, which AGENTS.md asks of every citation comment.Ana María García－Lópezkeeps familyGarcía－López, where it tookMaría. Every member of the class has a test row, held to a written-out list so dropping or adding one fails. No PSC record holds any of the four added characters, so decisions/O6's figures stand._idxhelper instead of re-implementing it.PolicyPatchcomment block is reflowed to its width.An independent review of the first commit found no wrong parse; its two findings (the hyphen class mostly untested, English names missing from the docstring's cost) are fixed in the second.
The default is untouched: the snapshot gains the new example text and changes no reading. pytest, mypy, ruff and both Sphinx builds pass.
🤖 Generated with Claude Code