Skip to content

fix(#459): repair one-case suffix acronyms to all-caps in case repair - #521

Merged
derek73 merged 5 commits into
derek73:masterfrom
IMGillusion:fix/459-suffix-acronym-caps
Sep 22, 2026
Merged

derek73 merged 5 commits into
derek73:masterfrom
IMGillusion:fix/459-suffix-acronym-caps

Conversation

@IMGillusion

@IMGillusion IMGillusion commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

What

HumanName('JOHN SMITH MBA').capitalize() returned 'John Smith Mba' — a credential acronym title-cased as an ordinary word (#459).

CAPITALIZATION_EXCEPTIONS only carries the entries that spell differently (md -> M.D., phd -> Ph.D., and the generational ii/iii/iv); every other suffix_acronyms entry fell through to str.capitalize() and got title-cased (mba -> Mba, jd -> Jd, qc -> Qc, mp -> Mp).

Fix

In _cap_word, after the exceptions-map lookup and before the Mac/Mc rule, a word whose period-free form is a known suffix_acronyms entry repairs to word.upper().

Gated on role is Role.SUFFIX so a word that is in the vocabulary but parsed as a family name (anh van do -> Anh Van Do) still repairs as an ordinary name word, not an acronym. The exceptions map is consulted first, so md -> M.D. and phd -> Ph.D. are unchanged.

Out of scope

The issue's QC MP example: the parser assigns QC the GIVEN role and MP the SUFFIX role, so only MP repairs to caps here and the given-name half stays title-cased, because the gate is keyed on the SUFFIX role rather than on "this word is an acronym". The gate does NOT, however, prevent the ambiguous five (ba, do, ed, jd, ma, all in suffix_acronyms) from being uppercased when they parse in the SUFFIX role: john smith ed -> John Smith ED, john smith ba -> BA, and smith, ms. -> MS. on the default path. That is the same #342/#454-class cost the issue already accepts, and the alternative (reading classify's vocab:suffix tag instead of the role) was measured and costs jd -> Jd, so the role gate is the right instrument. The letter-mask design (phd -> PhD, bsc -> BSc), md leaving the map, and the given-role half of QC MP stay on the rescoped #459.

Docs & tests

  • New examples in tests/test_capitalization.py (all-caps suffix, exceptions still win, family-name-in-vocab stays title-case).
  • rules.md R4: added a john smith mba example and the john smith jr boundary; regenerated tools/differential/corpus_rules.jsonl.
  • docs/customize.rst: the capitalization_exceptions demo word swapped from dds (now handled by default) to dphil (unshipped, the caller-added case Case repair: letter masks for the exceptions map (phd → PhD, bsc → BSc), md leaves the map, and roman numerals past iv (john smith xi → Xi) #459 names); the phd -> PhD parenthetical corrected to Ph.D..
  • ph left suffix_acronyms (it existed only to prop up the spaced Ph. D., which the new clause would read all-caps on the default path); the 13 PH. D. force movers go back to Ph. D., and phd/ph.d. still give Ph.D..
  • docs/design/decisions.md R4 entry and docs/release_log.rst 2.3.0 bullet.

Full suite: 7610 passed, 179 skipped, 4 xfailed.


Maintainer fix-up (2026-09-22, derek73)

Three commits pushed onto the branch after the contributor handed it over (76bcc86 merge from master, 9eec6b4 docs fix-up, 16b66c6 the ph parity-break record); the two contributor commits are unchanged ancestors. Current measurements supersede the paragraph above: on the merged tree, 1338 distinct corpus names, 178 carry an acronym suffix token, 30 single-case, 22 move on the default capitalize() path and 119 under force=True; recipe in the R4 entry of docs/design/decisions.md. The ph removal is classified as a 2.x parity break on the bare-ph-with-no-D. class (John Smith Ph. → last Ph.), pinned by a tests/v2/cases.py row and a rule in all five ledgers. Full suite: 9675 passed, 324 skipped, 4 xfailed; differential gate exit 0 at all five baselines, 0 unexplained.

🤖 Generated with Claude Code

Gated on the SUFFIX role; exceptions map consulted first so md/phd keep their special casing. R4 example added; corpus regenerated.

@derek73 derek73 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this -- it's a well-formed fix and the design is accepted as-is: all-caps repair for a word the parse put in the suffix role from the acronym vocabulary, exceptions map consulted first, placed before the Mac/Mc rule (which matters: mcse would otherwise come out McSe). The role gate is the right instrument and the right width; anh van do staying Do is exactly the control we want. The wider design in #459 (letter masks, md leaving the map, the given-role half of QC MP) stays on #459, which I'll rescope rather than close, so please read the deferrals below as scoping, not rejection.

Measured on the PR head over the differential corpora (1143 names): 5 move on the default capitalized() path, 71 under force=True; 122 names carry acronym vocabulary in a suffix token and 11 of those are single-cased, so the honest reach is 5 of 11 eligible.

Blocking (two behavior items)

  1. CI fails on the Sphinx doctest: docs/customize.rst around line 166 expects str(parse("jane smith dds").capitalized()) to be 'Jane Smith Dds'. That passage demonstrates capitalization_exceptions on a word your fix now handles by default, so the example loses its contrast even once the string is corrected. Please swap the demo word for one that is not in suffix_acronyms -- dphil is unshipped today and is the very word #459 names as the caller-added case, so the doc and the deferred design point at the same example. While there, the parenthetical two lines above ("phd" -> "PhD") has been wrong for a while: the shipped map gives Ph.D..

  2. ph needs to leave suffix_acronyms in this PR. The fragment exists only to prop up the spaced Ph. D., and with your clause it now reads all-caps on the default path, not only under force: john smith ph. d. -> John Smith PH. D., smith, ph. d. -> Smith PH. D.. Measured with ph removed: 0 corpus names move on any role field, 0 move under capitalized(), and the 13 PH. D. force movers go back to Ph. D.; john smith phd and ph.d. still give Ph.D.. The 2.3.0 release log already carries three bullets of this shape (rai, cha, esq leaving the acronym set under decisions.md#suffix-acronym-collisions), so it's a one-line addition there, not new scope.

Blocking (house conventions -- see AGENTS.md's same-PR amendment rule)

  1. rules.md#R4's statement doesn't yet say what your example line illustrates: the rule text covers the casing a vocabulary entry records and the Mac/Mc convention, and an all-caps repair keyed on the suffix role is a new conjunct. One sentence in the statement, plus a boundary example line "john smith jr" -> capitalized="John Smith Jr" (the boundary #459 itself states; the rules-doc runner executes it on the core path, which also covers the "is there a core-path test?" question).

  2. A docs/design/decisions.md bullet under the R4 entry: the decision and its narrow scope, the reach with population first (5 of 11 eligible, 122 in the population, 71 forced), a recompute recipe that names the comparator (the pre-change _cap_word from git show d37b8ec:nameparser/_render.py, swapped in-process), and the accepted costs -- see item 6.

  3. A docs/release_log.rst 2.3.0 Behavior Changes bullet in the house style: bold lead, before/after (HumanName("JOHN SMITH MBA").capitalize() gives John Smith MBA where every release since 1.4.0 gave John Smith Mba; jd -> JD; QC MP -> Qc MP with the given-name half unchanged; md/phd unchanged; jr untouched), provenance by entry name, and (#459) rather than closes #459.

Corrections to prose, non-blocking but they should land

  1. The comment says every other suffix_acronyms entry "is all-caps by definition" and that the map's five "need non-all-caps spelling". bsc and msc are conventionally mixed-case (Dr. med. univ. Margit Popp, MSc is a corpus name and now reads MSC under force), and ii/iii/iv need all-caps and are suffix_words, not acronyms. The clause is still the right default for an initialism; the justification just needs to say so and name the mixed-case cost as deferred. Likewise the PR description's ed -> ED argument: the role gate doesn't prevent it, it moves which parse triggers it -- john smith ed -> John Smith ED, john smith ba -> BA, smith, ms. -> MS. on the default path, because the ambiguous five (ba, do, ed, jd, ma) are in suffix_acronyms. That's an accepted cost of the #342/#454 class #459 already accepts, and the alternative (reading classify's vocab:suffix tag) was measured and costs jd -> Jd, so your choice is the right one; the decisions bullet should record the fork with those numbers. Also a stray extra blank line in the class body of tests/test_capitalization.py.

Deferred to the rescoped #459, so nobody relitigates them here: letter masks (BSC/MSC -> BSc/MSc), md leaving the map, QC in the given role, and the within-run inconsistency where vocabulary coverage shows (John Doe CLU, CFP, Lutc).

…ules/decisions/release-log, correct the comment

- docs/customize.rst: dds->dphil demo (dds now handled by default); phd parenthetical -> Ph.D.
- config/suffixes.py: ph left suffix_acronyms (read all-caps on the default path; 13 force movers back to Ph. D.)
- rules.md R4: all-caps clause sentence + john smith jr boundary line; regenerated corpus_rules.jsonl
- decisions.md R4: derek73#459 bullet (scope, reach 5/11-71, recompute recipe, accepted costs)
- release_log.rst 2.3.0: behavior bullet
- _render.py comment: bsc/msc mixed-case + ii/iii/iv are suffix_words; test blank line
@codecov

codecov Bot commented Sep 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.79%. Comparing base (23e52dc) to head (16b66c6).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #521   +/-   ##
=======================================
  Coverage   98.78%   98.79%           
=======================================
  Files          45       45           
  Lines        3636     3638    +2     
=======================================
+ Hits         3592     3594    +2     
  Misses         44       44           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@derek73

derek73 commented Sep 16, 2026

Copy link
Copy Markdown
Owner

Thanks -- this addresses everything I asked for, and the fix itself is unchanged from what I approved. What's left is docs-only, plus a rebase, because 2.3.0 shipped on the 12th and master moved 40 commits under you. I re-ran the suite and the differential gate on your two commits rebased onto current master: 7978 passed, and the gate exits 0 at all five baselines with 0 unexplained diffs, so the ph removal moves nothing on any role field, as measured.

If you'd rather not do a third round on house conventions, say so and I'll push these to your branch myself (as a merge from master plus one docs commit, no force-push, and I'll leave a comment saying what landed). Either way is fine with me.

Mechanical

  1. Rebase or merge master. The only conflict is tools/differential/corpus_rules.jsonl; regenerate it with uv run python tools/differential/build_rules_corpus.py rather than resolving by hand. It picks up exactly your two new R4 rows.

  2. The release-log bullet is under 2.3.0 - September 12, 2026, which has shipped and is one of the releases the bullet says gave Mba. Move it to the 2.4.0 - Unreleased Behavior Changes section.

Corrections to the decisions.md R4 entry

  1. The reach digits predate your own ph removal. Running the entry's recipe on the head that carries the sentence gives 111 names in the population and 58 forced movers, not 122 and 71; re-adding ph reproduces 122 and 71 exactly, the 13 being the PH. D. shapes the same commit fixed. The 5 of 11 pair holds and is the part the argument needs. Please update the digits, say the population is 1176 distinct names on current master, and name the ph interaction so the next person running the recipe doesn't file the difference as a regression. The release-log bullet inherits the 71 and needs the same fix.

  2. "The five entries that need a non-all-caps spelling (md -> M.D., phd -> Ph.D., and the roman numerals)" -- II/III/IV are all-caps, and the entry says so itself two lines later. This is the misstatement item 6 of my last review corrected in the code comment, re-introduced here. The map's job is spellings str.capitalize() gets wrong, not spellings that aren't all-caps.

  3. ph leaving the set has no record. rai, cha and esq each carry an Excluded: bullet in decisions.md (see Excluded (SUFFIX_ACRONYMS -- esq, removed 2026-09-08 ...) for the shape) plus a release-log bullet, and that convention exists so a wordlist sweep that finds Ph. D. parsing doesn't put the entry back. One short Excluded: bullet saying ph was a fragment propping up the spaced Ph. D., that the merge keys on the PH/D regexes rather than acronym membership, and that with it gone the clause no longer reads PH. D. on the default path; and a one-line release-log bullet in the shape of the rai/cha one.

Corrections to the rules.md R4 statement

  1. "a suffix word that is not an acronym -- the generational jr, sr -- keeps its title case" is falsified by john smith ii -> John Smith II. The numerals are suffix words, not acronyms, and the map outranks both clauses. Add the "that the exceptions map does not carry" qualifier the acronym clause beside it already has.

  2. The statement lists the all-caps repair and the Mac/Mc convention with no ordering, and mcse is claimed by both. It reads MCSE only because of where you placed the clause. One clause in the statement ("consulted before the Mac/Mc convention") pins that, since rules.md can't name the branch order the way decisions.md does.

  3. Lower priority: R4's Accepted paragraph ("the clause reaches a part the parser read") now sits beside a second clause that behaves differently -- the role gate does repair a spliced replace(suffix="mba") to MBA. Naming which clause the paragraph means resolves it.

Also refresh the PR description's measurement paragraph and test count once you've done 3.

Everything else re-measured and held on the rebased tree: the anh van do control, ed/ba/ms., the jd tag fork, Ph. D. on both paths, and upper/lower spellings repairing alike over all 111 population names.

@IMGillusion

Copy link
Copy Markdown
Contributor Author

I'll take you up on that - stopping at this round. Please push the master merge and the docs commits onto my branch; I'll re-run the suite on my end and watch CI. Thanks for carrying the heavy part.

derek73 and others added 3 commits September 22, 2026 00:34
…nym-caps

# Conflicts:
#	docs/design/decisions.md
#	docs/design/rules.md
#	tools/differential/corpus_rules.jsonl
…nd the ph removal is on the record

Maintainer fix-up on PR derek73#521 after the merge from master, the eight
items of the 2026-09-16 review comment:

- decisions.md R4: the reach is remeasured on the merged tree with
  the recipe's comparator named (master's `_cap_word` at 23e52dc),
  and the two earlier snapshots are explained rather than replaced --
  the `ph` removal took 13 forced movers back to `Ph. D.`, and the
  rows PRs derek73#530/derek73#532/derek73#534 added account for the rest of the drift.
  The exceptions map's job is stated as the spellings
  `str.capitalize()` gets wrong, not "non-all-caps" (II/III/IV are
  all-caps); the precedence over Mac/Mc and the role-not-tag reach
  of the gate are recorded.
- decisions.md: an `Excluded (SUFFIX_ACRONYMS — ph)` block beside
  esq's, so a wordlist sweep does not put the fragment back.
- rules.md R4: the non-acronym clause is qualified by the exceptions
  map (`john smith ii` -> `II`), the acronym repair's precedence over
  the Mac/Mc convention is stated and pinned by a `john smith mcse`
  row (`McSe` without it), and the Accepted paragraph names which
  clause it is about and states the acronym repair's contrast on a
  spliced suffix.
- release_log.rst: the bullet moves from the shipped 2.3.0 section
  to 2.4.0 with the current digits, and `ph` leaving the set gets
  its own bullet in the rai/cha shape.
- corpus_rules.jsonl regenerated (three new R4 rows).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…h. is pinned and classified

The docs review of the fix-up found what the removal's measurement
could not: `ph` also carried a bare trailing `Ph.` -- no `D.` behind
it -- as a credential on every release since 1.4.0, and with the
entry gone `John Smith Ph.` reads middle Smith, family Ph. The
corpus had no name of that shape, so "0 role movers" was true and
blind, the case AGENTS.md warns of.

- tests/v2/cases.py: a shape-1 row carries `John Smith Ph.` at the
  positional reading, classified fix(derek73#459), beside the esq rows the
  same criterion decided.
- corpus_shapes.jsonl regenerated with it.
- all five ledgers: a `change(suffix-acronym-collisions) ph leaves
  the acronym set` rule, literal and exactly as wide as the diff;
  the gate explains that one name at every baseline with nothing
  unexplained.
- test_ledger_guards.py: the rule's corpus claim recorded per
  ledger, and its negative controls (`Ph. D.` in both positions,
  `phd`, `Ph.D.`).
- decisions.md `Excluded (SUFFIX_ACRONYMS — ph)` and the 2.4.0
  release-log bullet state the cost, the classification, and the
  add-back for a caller who wants the old reading.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@derek73

derek73 commented Sep 22, 2026

Copy link
Copy Markdown
Owner

Pushed three commits onto your branch, no force-push -- your two commits are untouched ancestors. What landed and why, keyed to the numbered items above:

76bcc86 -- merge master. Items 1 and 2's rebase half. Master had moved 80 commits; the conflicts were corpus_rules.jsonl (regenerated) and adjacent insertions in rules.md R4's example block and decisions.md's R4 entry, resolved by keeping both sides in date order.

9eec6b4 -- the docs fix-up. Items 2 through 8:

  • Release-log bullet moved to 2.4.0 (2), with the current digits.
  • decisions.md R4: reach remeasured on the merged tree with the recipe's comparator named -- 1338 distinct corpus names, 178 carry an acronym suffix token, 30 single-case, 22 move on the default path and 119 under force=True -- and the drift from your 122/71 explained rather than overwritten: the ph removal took 13 forced movers back, and master's Case and shape decide a bare ambiguous credential acronym (#289, #516) #530/fix(#531): a credential ending the given part after a family comma reads as one, and reports #532/fix(#533): a maiden clause ends at a trailing credential, and reports either way #534 corpus rows (single-case trailing MA) account for 15 of the 17 further default movers (3). The map's job is stated as the spellings str.capitalize() gets wrong (4). Precedence over Mac/Mc recorded (7), and the gate's role-not-tag reach on a spliced suffix (8).
  • rules.md R4: the jr/sr sentence qualified by the exceptions map, since john smith ii repairs to II (6); "outranks the Mac/Mc convention where a word fits both" stated and pinned by a john smith mcse row, which reads McSe without the clause (7); the Accepted paragraph names the all-particle clause and states the acronym repair's contrast (8).
  • ph gets an Excluded (SUFFIX_ACRONYMS -- ph) block beside esq's and a release-log bullet (5).

16b66c6 -- one thing the review of the fix-up found that neither of us had. ph also carried a bare trailing Ph. -- no D. behind it -- as a credential on every release since 1.4.0 (wheels checked): John Smith Ph. gave suffix Ph. and now gives middle Smith, last Ph.. No corpus name had that shape, which is why "0 role movers" was true and blind. So it is now pinned and classified the way esq's removal was: a tests/v2/cases.py row carries the shape, all five ledgers carry a change(suffix-acronym-collisions) ph leaves the acronym set rule that explains exactly that one name at every baseline, the guard test records the claim and its negative controls (Ph. D. in both positions, phd, Ph.D.), and the decisions entry and release bullet state the cost and the add-back (Lexicon.default().add(suffix_acronyms={"ph"})). The class is a little wider than that one name, and the entry lists it: the undotted John Smith Ph/PH, Ph. inside a credential run (John Smith MD Ph. reads middle Smith MD, last Ph., taking the MD with it), and Smith, Ph. from 2.2.0 on. Every one is a bare ph with no D. behind it, which nobody writes -- same criterion as esq, so the removal stands; the ledger rule is literal to the pinned name and says to widen it if another shape of the class ever arrives.

Verified on the pushed head: 9675 passed / 324 skipped / 4 xfailed; differential gate exit 0 at all five baselines with 0 unexplained; Sphinx doctests pass; the design-docs review ran twice over the docs/design changes and its findings are what 16b66c6 fixes.

I appended a short maintainer section to the PR description with the current numbers rather than editing your text. Once CI is green on your side I'll merge.

@derek73 derek73 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved after the maintainer fix-up (76bcc86, 9eec6b4, 16b66c6): suite, differential gate at all five baselines, doctests and the design-docs review are green on 16b66c6.

@derek73
derek73 merged commit 4d0680e into derek73:master Sep 22, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants