Skip to content

Commit 8cb62a9

Browse files
derek73claude
andauthored
fix: correct suffix boundary lookup for prefixed last names (#100) (#179)
* fix: correct suffix boundary lookup for prefixed last names (#100) The prefix-joining loop located the suffix stop boundary with a value-based pieces.index() that searched from position 0. When a token value repeated (a trailing title that is also a suffix acronym, e.g. the second 'dr' in 'dr Vincent van Gogh dr'), it matched the leading occurrence, producing an empty slice that duplicated pieces and corrupted the middle name. Constrain the lookup to start at i + 1, consistent with the sibling next_prefix lookup. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test: guard against #108 exponential blow-up on repeated prefixes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * review: improve comment accuracy and test coverage - Fix inline comment in join_on_conjunctions: clarify that filter() finds the value in pieces[i+1:] but index() searches from 0 by default, and drop the misleading "title" framing (the token only needs to satisfy is_suffix, not is_title) - Add test for two-word prefix collision ("van der") — different loop iteration count than the single-word case - Add test with a genuine middle name alongside the repeated token, since the pre-fix bug corrupted the middle field specifically - Add @pytest.mark.timeout(2) to the #108 guard so the timeout is enforced locally and in CI, not just by CI job limits - Assert hn.last contains "Berg" in the #108 guard to catch silent last-name corruption - Add pytest-timeout dev dependency - Resolve pre-existing stash conflict in docs/resources.rst (keep upstream) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent f14fb58 commit 8cb62a9

4 files changed

Lines changed: 120 additions & 54 deletions

File tree

nameparser/parser.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -980,7 +980,12 @@ def join_on_conjunctions(self, pieces: list[str], additional_parts_count: int =
980980
try:
981981
# if there are no more prefixes, look for a suffix to stop at
982982
stop_at = next(iter(filter(self.is_suffix, pieces[i + 1:])))
983-
j = pieces.index(stop_at)
983+
# search from i + 1: filter() finds the value of stop_at
984+
# in pieces[i+1:] but pieces.index() without a start
985+
# argument searches from 0, so an earlier occurrence of
986+
# the same token (e.g. a suffix token that also appears
987+
# before the prefix) would be matched instead.
988+
j = pieces.index(stop_at, i + 1)
984989
new_piece = ' '.join(pieces[i:j])
985990
pieces = pieces[:i] + [new_piece] + pieces[j:]
986991
except StopIteration:

pyproject.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ dev = [
4343
"dill (>=0.2.5)",
4444
"sphinx (>=8)",
4545
"mypy (>=2.1)",
46-
"ruff (>=0.15)"
46+
"ruff (>=0.15)",
47+
"pytest-timeout>=2.4.0",
4748
]
4849

4950
[tool.mypy]

tests/test_prefixes.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import pytest
2+
13
from nameparser import HumanName
24

35
from tests.base import HumanNameTestBase
@@ -43,6 +45,50 @@ def test_prefix_before_two_part_last_name_with_acronym_suffix(self) -> None:
4345
self.m(hn.last, "von bergen wessels", hn)
4446
self.m(hn.suffix, "M.D.", hn)
4547

48+
def test_title_before_and_after_prefixed_last_name(self) -> None:
49+
# Issue #100: a repeated title/suffix token ("dr") before AND after a
50+
# prefixed last name used to corrupt the middle name into
51+
# " dr Vincent van" because the suffix-boundary lookup matched the
52+
# LEADING "dr" instead of the trailing one.
53+
hn = HumanName("dr Vincent van Gogh dr")
54+
self.m(hn.title, "dr", hn)
55+
self.m(hn.first, "Vincent", hn)
56+
self.m(hn.middle, "", hn)
57+
self.m(hn.last, "van Gogh", hn)
58+
self.m(hn.suffix, "dr", hn)
59+
60+
def test_suffix_token_collision_with_two_word_prefix(self) -> None:
61+
# Same fix as #100 but with a two-word prefix ("van der"). Exercises a
62+
# different iteration count through the prefix-joining loop.
63+
hn = HumanName("dr Vincent van der Gogh dr")
64+
self.m(hn.title, "dr", hn)
65+
self.m(hn.first, "Vincent", hn)
66+
self.m(hn.middle, "", hn)
67+
self.m(hn.last, "van der Gogh", hn)
68+
self.m(hn.suffix, "dr", hn)
69+
70+
def test_title_before_and_after_prefixed_last_name_with_middle(self) -> None:
71+
# The pre-fix bug corrupted the middle field; verify it is not disturbed
72+
# when a genuine middle name is present alongside the repeated token.
73+
hn = HumanName("dr Vincent James van Gogh dr")
74+
self.m(hn.title, "dr", hn)
75+
self.m(hn.first, "Vincent", hn)
76+
self.m(hn.middle, "James", hn)
77+
self.m(hn.last, "van Gogh", hn)
78+
self.m(hn.suffix, "dr", hn)
79+
80+
@pytest.mark.timeout(2)
81+
def test_many_repeated_prefixes_does_not_blow_up(self) -> None:
82+
# Issue #108: a name with a long run of repeated prefixes used to grow
83+
# the pieces list exponentially and exhaust memory. The 2-second timeout
84+
# enforces this locally and in CI — if the test hangs, an exponential
85+
# regression has been reintroduced.
86+
name = "Jan " + "van der " * 30 + "Berg"
87+
hn = HumanName(name)
88+
self.assertFalse(hn.unparsable)
89+
self.m(hn.first, "Jan", hn)
90+
self.assertIn("Berg", hn.last)
91+
4692
def test_two_part_last_name_with_suffix_comma(self) -> None:
4793
hn = HumanName("pennie von bergen wessels, III")
4894
self.m(hn.first, "pennie", hn)

uv.lock

Lines changed: 66 additions & 52 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)