Skip to content

Commit 2f57ff2

Browse files
authored
Merge pull request #532 from derek73/claude/531-given-part-trailing-slot
fix(#531): a credential ending the given part after a family comma reads as one, and reports
2 parents cc78c96 + 03625a2 commit 2f57ff2

24 files changed

Lines changed: 2927 additions & 131 deletions

‎AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ Add a dedicated `copy.deepcopy()` round-trip test for it too (see `test_regexes_
380380

381381
**`_normalize` must reach a fixed point** — storage and match-time share the one fold, and `Lexicon.__setstate__` re-validates, so a value that changes on re-normalization changes under its owner. `strip().strip(".")` alone is not idempotent (`'. a .'` → `' a '` → `'a'`). The loop is the fix; keep any new stripping inside it. **Anything built on `_normalize` must converge too** — `_fold_words` runs `_normalize` per word and DROPS the words that fold away (`_title_key` is that list space-joined, and `_run_addresses_by_given` reads the list itself, so its last-word arm is the last word of the FOLDED key by construction); keeping the empty slot stored `'lt .'` as `'lt '`, a key match-time can never rebuild (so the entry is silently inert) and `__setstate__` rejects on the next round-trip as "not written by this version".
382382

383-
**Perf regressions are caught by the scaling test, not the absolute-time ones** — `tests/v2/test_benchmark.py::test_parse_cost_grows_no_worse_than_linearly` times a repeated unit at n vs 4n over ten shapes (one per pipeline inner loop) and bounds the ratio; the `_thousand_names` tests use constant-size, delimiter-free input and are structurally blind to a complexity regression. Two rules when touching it: calibrate `_MAX_RATIO` against the WEAKEST quadratic's signal (a mixed quadratic surfaces far below the textbook 16×, so the operating point `_BASE` matters more than the bound), and confirm a planted regression fails it across REPEATED runs — one failure is a coin-flip on a timing test. The ten shapes cover different dimensions (segment count only via `commas`, intra-piece accumulation only via `particles`/`conjunctions`, non-ASCII input only via `honorifics` — the other nine are pure ASCII, so `script_segment` returns at its bail and the CJK stages go unmeasured); measure before pruning one. A stage gated on an opt-in `Policy` field needs a `_POLICY_SHAPES` entry instead, since bare `parse()` never enters it — and that table's rows carry a **reachability probe** run before the measurement, because a precedence change can quietly stop the shape reaching the stage and leave a green test measuring a no-op (`_POLICY_SHAPES` is also asserted non-empty: an empty `parametrize` is a skip, not a failure, so deleting its last row would retire the guard silently).
383+
**Perf regressions are caught by the scaling test, not the absolute-time ones** — `tests/v2/test_benchmark.py::test_parse_cost_grows_no_worse_than_linearly` times a repeated unit at n vs 4n over eleven shapes (one per pipeline inner loop) and bounds the ratio; the `_thousand_names` tests use constant-size, delimiter-free input and are structurally blind to a complexity regression. Two rules when touching it: calibrate `_MAX_RATIO` against the WEAKEST quadratic's signal (a mixed quadratic surfaces far below the textbook 16×, so the operating point `_BASE` matters more than the bound), and confirm a planted regression fails it across REPEATED runs — one failure is a coin-flip on a timing test. The eleven shapes cover different dimensions (segment count only via `commas`, intra-piece accumulation only via `particles`/`conjunctions`, non-ASCII input only via `honorifics` — the other ten are pure ASCII, so `script_segment` returns at its bail and the CJK stages go unmeasured); measure before pruning one. **A shape the CLOCK cannot reach needs a FRAME-count guard instead**, which is the second scaling test in that file (`test_a_trailing_credential_run_does_not_cost_exponentially`, #531): where the defect is an exponential rather than a quadratic, the input length that separates the curves on a timing test does not finish, so the guard counts frames over 8 units against 16 and bounds THAT ratio. One pair does not see every curve, and the fix round for #531 measured why: at 2× the input the per-member LINEAR work swamps a quadratic (2.08× for a genuine one against 1.73× clean), so that pair guards the exponential alone and a second, longer pair — 16 against 64, where the same quadratic reads 7.42× against 3.53× clean — is what can see one. Assert them in that order: an exponential never returns from the longer run, so the cheap pair has to have failed first. Frame counts do not move under load, so this shape needs no repeated-run calibration — but it does need the same reachability assertion `_POLICY_SHAPES` rows carry, since the walk under measurement runs only while every unit still reads as a credential. A stage gated on an opt-in `Policy` field needs a `_POLICY_SHAPES` entry instead, since bare `parse()` never enters it — and that table's rows carry a **reachability probe** run before the measurement, because a precedence change can quietly stop the shape reaching the stage and leave a green test measuring a no-op (`_POLICY_SHAPES` is also asserted non-empty: an empty `parametrize` is a skip, not a failure, so deleting its last row would retire the guard silently).
384384

385385
**Expected-failure tests use `@pytest.mark.xfail`** — the conftest parametrized fixture breaks `@unittest.expectedFailure`; always use `@pytest.mark.xfail` instead.
386386

‎docs/customize.rst‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,10 @@ listed below.
452452
as a credential where the position allows it:
453453
``"John Smith X.Y.Z."`` gives suffix ``X.Y.Z.`` while
454454
``"Jack X.Y.Z."`` keeps family ``X.Y.Z.``, and either reading
455-
is reported. Case is irrelevant — the periods are the signal.
455+
is reported. The family-comma form is one of those positions
456+
since 2.4: ``"Doe, John X.Y.Z."`` gives suffix ``X.Y.Z.``
457+
while ``"Doe, X.Y.Z."`` keeps given ``X.Y.Z.``.
458+
Case is irrelevant — the periods are the signal.
456459
Whole-token vocabulary still wins (``M.A.``, ``Ph.D.``), and a
457460
single trailing period is not this shape
458461
(``"John Smith Xyz."`` keeps family ``Xyz.``). Two further
@@ -474,7 +477,9 @@ listed below.
474477
- Reads an unlisted all-caps word of two or more letters, with no
475478
period in it, in a name written in more than one case as a
476479
credential where the position allows it: ``"John Smith XYZ"``
477-
gives suffix ``XYZ``. Defaults to ``False``, and deliberately:
480+
gives suffix ``XYZ``, and since 2.4 so does the family-comma
481+
form ``"Doe, John XYZ"``. Defaults to ``False``, and
482+
deliberately:
478483
an all-caps surname is a real writing convention that shape
479484
cannot separate from a credential, so ``"Jean Pierre DUPONT"``
480485
gives family ``Pierre``, suffix ``DUPONT`` with this on. Off,

‎docs/design/decisions.md‎

Lines changed: 4 additions & 0 deletions
Large diffs are not rendered by default.

‎docs/design/mechanisms.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,4 +197,5 @@ Problem shape. A test pins an ordering, a sort, a dedup or a partition, and its
197197
- A detector that re-implements a rule's grouping will get the grouping wrong. Derive the boundary from the same vocabulary the rule reads, not from the half you happen to be thinking about: walking a particle run over the NEVER-GIVEN set alone (the rule chains through ANY particle) split "de la Vega" after "de la" and reported 50 false movers for #364, where the true count is one. Both wrong answers were plausible and printed cleanly.
198198
- Guard the whole family, parametrize over it: a defect on one of N parallel entry points hides behind a per-example test — three times in one session (a guard on one class of two, a decode hint on 3 of 5 entry points, a sync roster missing 4 copies) — and a {class}×{field}×{bad-value} parametrization is what caught each.
199199
- A growth guard needs calibration, not just existence: benchmark guards that compare n vs 4n catch the quadratic the absolute-time tests are blind to, but calibrate against the WEAKEST signal you must detect and confirm a planted regression fails across repeated runs, not once — a stochastic check "verified" on one sample verifies nothing.
200+
- A frame BUDGET pinned on one reference name cannot see algorithmic blowup; a frame RATIO over two input sizes can. #531's first implementation had a trailing walk re-enter the predicate that owns it, so a run of k ambiguous credentials cost 2**k — `'Doe, John ' + 'MA '*24` took about 6s, wall clock, where the parent commit took 0.24ms (two runs of that one measurement, 5.8s and 5.9s, which is the spread the guard below quotes them at) — while every absolute figure beside the change stayed correct and unmoved: `tools/perf/call_count.py`'s 412/449 on its reference name, and +0 frames on a comma name carrying no class member. The budget is a claim about ONE input and the defect is a claim about a family of them, so no budget is the instrument. The guard that catches it counts frames at 8 members against 16 and bounds THAT ratio (`tests/v2/test_benchmark.py::test_a_trailing_credential_run_does_not_cost_exponentially`): 2,347 → 394,671 frames unfixed, 168x, against 865 → 1,497 and 1.73x fixed, with the bound at 6x. ONE pair does not see every curve, though, and the fix round measured how badly: at 2× the input the per-member linear work swamps a quadratic, so the per-member memo the first fix shipped — a genuine quadratic — reads 2.08x at that pair, inside any bound this one can carry. Separating those needs 4× the input, where the same memo reads 7.42x against the linear 3.53x, so the guard carries a second pair (16 against 64) and asserts the cheap one FIRST: an exponential does not return from a run of 64. Frames rather than a clock for two reasons — the recursion IS frame entries, so the count is the defect rather than a proxy, and the input length that separates an exponential from a linear on a timing test does not finish. It needs the reachability assertion a `_POLICY_SHAPES` row carries, for the same reason: the walk under measurement runs only while every unit still reads as a credential, and a precedence change that routes them elsewhere leaves a green test measuring a walk that no longer happens.
200201
- Mind the optional-extra environment split: a local venv's incidental namedivider makes `if available` branches run PRESENT locally and ABSENT in CI, so a locally-green suite proves nothing about the no-extra path (this broke #337's first landing). Run the decisive check in both states or gate the example.

0 commit comments

Comments
 (0)