Skip to content

Commit a527bd6

Browse files
committed
Pin the v1 shim's honorific_tails translation (#308)
honorific_tails appeared nowhere in tests/v2/test_config_shim.py, and the snapshot-equality test passes VACUOUSLY here -- with the default config the GLUED_HONORIFICS ∩ suffix_words intersection is a no-op, which is exactly the failure mode AGENTS.md warns about. Pin the case the translation decides: deleting a CJK honorific from Constants.suffix_not_acronyms, the only v1 knob that reaches the field, turns the peel off rather than raising the subset error or leaving a tail that peels into a suffix field no longer recognizing it. AGENTS.md's roster of shim transformations said four and named four; this branch added the fifth. Same-commit rule applied one commit late.
1 parent cd626cb commit a527bd6

2 files changed

Lines changed: 21 additions & 1 deletion

File tree

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ The 2.0 rewrite lands as underscore-private modules alongside the v1 code. These
131131
- **Parser owns config-dependent conveniences**: `Parser.matches`/`Parser.capitalized`/`Parser.revise` exist because the `ParsedName` equivalents fall back to DEFAULT config for str/omitted arguments (documented loudly in both docstrings). `revise` harvests tokens from a full sub-parse of each replacement value (tags kept minus `FOLDED_TAG`, roles forced, ambiguities discarded); the merge tail is shared with `replace()` via `ParsedName._with_field_tokens`. `Parser.capitalized` delegates through `name.capitalized(self.lexicon)` specifically so `_parser` never imports `_render` — keep it that way.
132132
- **Per-word vocabulary fields warn on multi-word entries** (`_normset`/`_normpairs` via `_warn_dead_entry`, UserWarning, never a raise — see the given_name_titles Gotcha for why raising is wrong). `given_name_titles` is the one multi-word-matched field and is exempt; `_edit` passes `warn=False` (add() warns once via the new instance's `__post_init__`; remove() stores nothing). The default vocabulary and every locale pack must stay warning-free (`test_default_lexicon_builds_warning_free`, `test_pack_vocabulary_entries_are_single_words`).
133133
- **Invariants guard harm, not no-ops**: add a constructor check when violating it produces a *wrong parse*, not when it produces *nothing*. A false positive costs a working configuration; a true positive on an inert condition costs the user nothing, so that trade is never worth taking. `suffix_acronyms_ambiguous ∩ suffix_words` is guarded because the overlap loses a family name; `given_name_titles` is not, because an unreachable entry is simply never consulted (see Gotchas). Before adding one, construct the config it forbids and check what actually breaks.
134-
- **The shim TRANSLATES; it never raises on a config v1 accepted, and never silently changes the parse**: `Constants._snapshot()` is a translation boundary between v1's model and v2's invariants, and every transformation there carries its v1-reachability argument in a comment. Four exist today — `first_name_titles` re-folded per word (v1 joins-then-`lc`, v2 normalizes-then-joins), `suffix_acronyms_ambiguous ∩ acronyms` (a provable no-op), `suffix_words − ambiguous` (v1 already accepts the word via the acronym branch, so the addition is inert there), and `particles_ambiguous ∪ (bound ∩ particles)` (a pinned deviation, `test_bound_never_given_prefix_deviates_on_two_pieces`). When a v1 config cannot satisfy a v2 invariant, work out what v1 actually *does* with it — usually nothing — and reproduce that; weakening the invariant or letting the raise through are both wrong. **Test the case the translation decides**, not one where both branches agree: a test using an input v1 parses identically with and without the config pins nothing.
134+
- **The shim TRANSLATES; it never raises on a config v1 accepted, and never silently changes the parse**: `Constants._snapshot()` is a translation boundary between v1's model and v2's invariants, and every transformation there carries its v1-reachability argument in a comment. Five exist today — `first_name_titles` re-folded per word (v1 joins-then-`lc`, v2 normalizes-then-joins), `suffix_acronyms_ambiguous ∩ acronyms` (a provable no-op), `suffix_words − ambiguous` (v1 already accepts the word via the acronym branch, so the addition is inert there), `particles_ambiguous ∪ (bound ∩ particles)` (a pinned deviation, `test_bound_never_given_prefix_deviates_on_two_pieces`), and `honorific_tails = GLUED_HONORIFICS ∩ suffix_words` (#308 behavior with no v1 manager of its own, so the one v1 knob that reaches it is deleting the suffix word — which turns the peel off, `test_snapshot_removing_a_honorific_word_turns_the_peel_off`). When a v1 config cannot satisfy a v2 invariant, work out what v1 actually *does* with it — usually nothing — and reproduce that; weakening the invariant or letting the raise through are both wrong. **Test the case the translation decides**, not one where both branches agree: a test using an input v1 parses identically with and without the config pins nothing.
135135
- **Reprs are bounded**: render which fields deviate from a named baseline and by how much, never contents (`Lexicon(default + titles: +2)`). `PolicyPatch`'s repr shows only set (non-UNSET) fields; `_order_repr` must never raise even on an unvalidated patch's garbage `name_order` (PolicyPatch defers validation to apply time); the sweep test in `tests/v2/test_reprs.py` pins that no config repr leaks the UNSET sentinel.
136136
- **Typing/docs**: `from __future__ import annotations`; `frozen=True, slots=True` on every public dataclass; strict-profile mypy flags via per-module overrides in pyproject (`strict = true` itself is not valid per-module). Docstrings state contracts in prose with **no doctest blocks**`--doctest-modules` makes every example a test; behavior examples go to unit tests per the lean-docs rule. **Document the positive direction of a partial property**: "a non-empty `ambiguities` is a signal to act on" is checkable, while "an empty one means no fork occurred" is a universal negative needing exhaustive verification -- that claim was written twice and falsified twice, at sites the author had not audited.
137137
- **The segmenter contract**: the optional `Parser(segmenter=...)` hook is parse-totality's ONE exception (locales spec section 4). Everything inside that exception is a bug in USER CODE, never a fact about the name, so it is surfaced rather than absorbed: the segmenter's own exceptions propagate, and the two protocol violations the stage can detect for itself — an answer of the wrong type, and one cutting at or past the end of the token it was handed — raise `TypeError`/`ValueError` from `_script_segment` for the same reason. The line to hold when adding a check there: a protocol violation by the segmenter's AUTHOR raises, while an adapter's defense against its own third-party library (`locales/ja.py`'s repertoire, length, reconstruction and score guards) declines with `None`, because what those catch is a fact about the content.

tests/v2/test_config_shim.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,26 @@ def test_snapshot_keeps_a_bound_never_given_prefix_parseable() -> None:
476476
assert (name.first, name.last) == ("dos Santos", "Silva")
477477

478478

479+
def test_snapshot_removing_a_honorific_word_turns_the_peel_off() -> None:
480+
# The deciding case for honorific_tails, which has no v1 manager of
481+
# its own: the snapshot intersects GLUED_HONORIFICS with the WORD
482+
# set, so deleting 씨 from suffix_not_acronyms -- the only v1 knob
483+
# that reaches it -- must make the tail stop mattering rather than
484+
# raise the subset error or leave 씨 peeling into a suffix field
485+
# that no longer recognizes it. With the default config the
486+
# intersection is a no-op, so the equality test above pins nothing
487+
# here.
488+
c = Constants()
489+
assert HumanName("김민준씨", constants=c).suffix == "씨" # baseline
490+
c.suffix_not_acronyms.remove("씨")
491+
lexicon, _, _ = c._snapshot() # must not raise
492+
assert "씨" not in lexicon.honorific_tails
493+
# the peel is off: the glued honorific goes back into the name,
494+
# which is 2.0's answer for this input
495+
name = HumanName("김민준씨", constants=c)
496+
assert (name.first, name.last, name.suffix) == ("민준씨", "김", "")
497+
498+
479499
def test_snapshot_field_translation() -> None:
480500
c = Constants()
481501
lexicon, policy, defaults = c._snapshot()

0 commit comments

Comments
 (0)