Skip to content

Should Parser.revise(name, suffix=name.suffix) be the identity? (John Smith MD PhD gives MD PhD, and revising with that string gives MD, PhD back) #511

Description

@derek73

Rationale

Parser.revise(name, suffix=...) classifies the new value by a full sub-parse of the bare string and then gives every harvested token the named role. Its docstring says the value "is classified ON ITS OWN", and that is the limit this issue is about: a suffix string carries no comma the entry rule can route by, and the sub-parse reads its words as a name, so the entry structure #436 derives from the written text is never derived for a revised field.

The user-visible shape is that feeding a name's own suffix back is not the identity:

p = parse("John Smith MD PhD")
p.suffix                                   # 'MD PhD'
Parser().revise(p, suffix=p.suffix).suffix # 'MD, PhD'

Nor is the within-piece heal kept:

p = parse("John Smith, Ph. D.")
Parser().revise(p, suffix=p.suffix).suffix # 'Ph., D.'   (was 'Ph. D.')

And a delimiter core comes back as an entry of its own:

p = parse("Doe, John, MD PhD - FACS Fellow")
Parser().revise(p, suffix=p.suffix).suffix # 'MD, PhD, -, FACS, Fellow'

Measurement (2026-09-06, master at 330ee55)

Over the 1117 distinct names in tools/differential/corpus*.jsonl, revise(p, suffix=p.suffix).suffix != p.suffix for 38. 36 are a space-joined run coming back comma-joined; the other two are the Ph., D. split and the dash-as-entry above. Before #510 it was 24 of 1116: #510 made more suffix views space-joined, so more of them now differ from what the sub-parse reconstructs. The parse side is unchanged — revise never round-tripped these, and the release-log bullet for #436 says so.

RECOMPUTE:

from nameparser import Parser
P = Parser()
bad = [n for n in corpus_names
       if (p := P.parse(n)).suffix
       and P.revise(p, suffix=p.suffix).suffix != p.suffix]

Why the sub-parse cannot get it right as it stands

The sub-parse of "MD PhD" is a full pipeline over a two-word string with no comma: MD reads as a given name and PhD as its suffix, so the #436 pass sees one SUFFIX token and joins nothing. revise then forces both tokens to SUFFIX, after the pass has run, and _with_field_tokens re-runs nothing. The joined tag is the persisted form of an entry (mechanisms.md#MARK-DONT-STRIP; _facade.__setstate__ re-derives it from the entry strings on unpickle), and nothing in revise writes it.

Options

  1. Re-derive the entries after the forced roles. Run the parse("John Smith MD PhD").suffix returns "MD, PhD" — a run without a comma renders with one #436 pass (rules.md#R1) over the harvested tokens once they carry the named role, using the sub-parse's own spans and comma offsets. The rule is the same one the main parse applies: a comma between two suffix words parts them, spaces join them. The within-piece heal needs the same treatment or the Ph. D. case stays split. Smallest change; keeps revise's "one string, one field" API.
  2. Accept a list. revise(p, suffix=["MD PhD", "FACS"]), one string per entry, the way suffix_list reads. Makes the entry structure explicit and sidesteps the sub-parse entirely, but widens the API and leaves the string form with the limit.
  3. Document the limit and leave it. revise already says the value is classified on its own; add the entry sentence and a test pinning the current values, so the next person does not rediscover it.

Option 1 is the honest fix if revise is meant to accept what the views produce; option 3 is fine if revise is meant for fresh input rather than round-trips. Either way the Ph., D. case is worth a pin, since it is the v1 fix_phd mechanism failing on the path nobody tested.

Where this is written down

Parser.revise's docstring (the "classified ON ITS OWN" limit), the C1 entry of docs/design/decisions.md (the NOT FIXED clause the #436 bundle added, with the 24 → 38 measurement), and the 2.3.0 release-log bullet for #436.

Activity

  1. self-assigned this
    on Sep 6, 2026
  2. added a commit that references this issue on Sep 7, 2026
  3. derek73 commented on Sep 7, 2026

    @derek73
    OwnerAuthor

    Fixed in #512, merged as 79160f3.

    What changed. Parser.revise now sub-parses each value to a pipeline state, forces the named role on every non-dropped token, and re-runs the R1 entry pass (suffix_entries, lifted out of the tail of post_rules so it can be called on its own) over the forced state. A suffix value's entries therefore come from its own commas, by the rule a whole name uses: a comma parts two credentials and a space joins them.

    • Parser().revise(n, suffix="MD PhD").suffix → MD PhD (was MD, PhD); "MD, PhD" stays two entries.
    • revise(n, suffix="Ph. D.") → Ph. D. (was Ph., D.). The Ph. D. merge is still a head-position rule and still does not fire in a value; the entry pass joins the pair because they share a comma bucket. The 2026-08-31 acceptance of Ph., D. is recorded as superseded under decisions.md#phd-merge.
    • revise(n, suffix="MD PhD - FACS Fellow") → one entry under the default policy (was five).

    Measurement. Over the 1117 distinct corpus names, 368 of which carry a suffix, revise(p, suffix=p.suffix).suffix != p.suffix went from 38 to 1. The one left is 김민준씨, J.씨: the whole-name parse keeps J.씨 one glued suffix token, while the bare value's sub-parse peels the honorific off the initial, so the revised field renders 씨, J. 씨 (was 씨, J., 씨). Right entries, one word read differently on its own — the "classified ON ITS OWN" limit the docstring records, and a W-rule question rather than an entry-structure one. Pinned as a limit; a corpus-wide guard (367 parametrized cases) now holds the property, since revise is off the differential gate's compare path.

    One more limit, pinned. A delimiter configured through extra_suffix_delimiters parts a value only where the value's own words read as a name with a tail segment, so in a run of post-nominals it stays a word ("MD PhD - FACS" and "MD, PhD - FACS" both keep the dash); write a comma at the boundary instead. The round-trip is unaffected, the whole-name view already rendering that boundary as a comma.

    Option 1 from the issue, as recommended. Option 2 (a list form) and the pins-only option were declined; the argument is in the #511 bullet under decisions.md#C1. ParsedName.replace() is unchanged.

  4. added this to the v2.3 milestone on Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions